Skip to content

[Merged by Bors] - chore: split Algebra/BigOperators/Group/{Multiset,Finset}#20629

Closed
kim-em wants to merge 17 commits intomasterfrom
split_bigoperators_group
Closed

[Merged by Bors] - chore: split Algebra/BigOperators/Group/{Multiset,Finset}#20629
kim-em wants to merge 17 commits intomasterfrom
split_bigoperators_group

Conversation

@kim-em
Copy link
Copy Markdown
Contributor

@kim-em kim-em commented Jan 10, 2025

Follow on from #20625.

Keeps only what is possible with the minimal imports required for the definitions themselves in the Defs file, and everything else moves to Lemmas.

kim-em and others added 8 commits January 10, 2025 12:22
@kim-em
Copy link
Copy Markdown
Contributor Author

kim-em commented Jan 10, 2025

Still needs a shake.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 10, 2025

PR summary 6dc03030e4

Import changes for modified files

Dependency changes

File Base Count Head Count Change
Mathlib.Data.Nat.GCD.BigOperators 512 451 -61 (-11.91%)
Mathlib.Algebra.Regular.Pow 521 460 -61 (-11.71%)
Mathlib.Algebra.BigOperators.RingEquiv 523 462 -61 (-11.66%)
Mathlib.Algebra.Star.BigOperators 578 518 -60 (-10.38%)
Mathlib.GroupTheory.Congruence.BigOperators 513 467 -46 (-8.97%)
Mathlib.Deprecated.Submonoid 524 480 -44 (-8.40%)
Mathlib.Algebra.Order.BigOperators.GroupWithZero.Multiset 425 390 -35 (-8.24%)
Mathlib.Algebra.Module.Submodule.LinearMap 590 548 -42 (-7.12%)
Mathlib.CategoryTheory.Preadditive.Basic 645 600 -45 (-6.98%)
Mathlib.Data.Rat.BigOperators 598 561 -37 (-6.19%)
Mathlib.Logic.Godel.GodelBetaFunction 555 525 -30 (-5.41%)
Mathlib.CategoryTheory.Subobject.Lattice 663 632 -31 (-4.68%)
Mathlib.RingTheory.Localization.Defs 688 659 -29 (-4.22%)
Mathlib.Algebra.Order.BigOperators.Ring.Multiset 463 444 -19 (-4.10%)
Mathlib.GroupTheory.QuotientGroup.Basic 658 655 -3 (-0.46%)
Import changes for all files
Files Import difference
There are 3873 files with changed transitive imports taking up over 168453 characters: this is too many to display!
You can run scripts/import_trans_difference.sh all locally to see the whole output.

Declarations diff

No declarations were harmed in the making of this PR! 🐙

You can run this locally as follows
## summary with just the declaration names:
./scripts/declarations_diff.sh <optional_commit>

## more verbose report:
./scripts/declarations_diff.sh long <optional_commit>

The doc-module for script/declarations_diff.sh contains some details about this script.


Increase in tech debt: (relative, absolute) = (1.00, 0.00)
Current number Change Type
3102 1 total LoC in Deprecated files

Current commit 6dc03030e4
Reference commit 039d0c7642

You can run this locally as

./scripts/technical-debt-metrics.sh pr_summary
  • The relative value is the weighted sum of the differences with weight given by the inverse of the current value of the statistic.
  • The absolute value is the relative value divided by the total sum of the inverses of the current values (i.e. the weighted average of the differences).

@leanprover-community-bot-assistant leanprover-community-bot-assistant added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Jan 10, 2025
@leanprover-community-bot-assistant leanprover-community-bot-assistant removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Jan 10, 2025
Copy link
Copy Markdown
Contributor

@Ruben-VandeVelde Ruben-VandeVelde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to fix the matrix test again; otherwise this seems sensible enough to me.

maintainer delegate

@github-actions
Copy link
Copy Markdown

🚀 Pull request has been placed on the maintainer queue by Ruben-VandeVelde.

@github-actions github-actions bot added the maintainer-merge A reviewer has approved the changed; awaiting maintainer approval. label Jan 10, 2025
@YaelDillies
Copy link
Copy Markdown
Contributor

Nice decrease! Can we get a few sentences explanation of the split in the PR description?

@leanprover-community-bot-assistant leanprover-community-bot-assistant added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Jan 11, 2025
@leanprover-community-bot-assistant leanprover-community-bot-assistant removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Jan 11, 2025
refine prod_bij' (fun x hx => ⟨x, hx⟩) (fun x _ ↦ x) ?_ ?_ ?_ ?_ ?_ <;> aesop

@[to_additive]
lemma prod_ite_of_false {p : α → Prop} {_ : DecidablePred p} (h : ∀ x ∈ s, ¬p x) (f g : α → β) :
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May I suggest that something is wrong if the ite and dite versions of the same lemma find themselves in different files?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I disagree that this is wrong, but have postponed the dite versions into the file with the ite versions.

Very happy if someone wants to go through and reduce the import requirements of proofs and then move lemmas upstream, but that's not on the agenda here.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What "agenda"? What's the greater context for reducing imports here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"not on the agenda" meaning I'm not interesting in rewriting proofs in this file during this PR.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm just trying to understand why you are doing this split 😅

@leanprover-community-bot-assistant leanprover-community-bot-assistant added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Jan 12, 2025
@kim-em kim-em removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Jan 13, 2025
@jcommelin
Copy link
Copy Markdown
Member

Thanks 🎉

bors merge

@ghost ghost added the ready-to-merge This PR has been sent to bors. label Jan 13, 2025
mathlib-bors bot pushed a commit that referenced this pull request Jan 13, 2025
Follow on from #20625.

Keeps only what is possible with the minimal imports required for the definitions themselves in the `Defs` file, and everything else moves to `Lemmas`.

Co-authored-by: Ruben Van de Velde <65514131+Ruben-VandeVelde@users.noreply.github.com>
@mathlib-bors
Copy link
Copy Markdown
Contributor

mathlib-bors bot commented Jan 13, 2025

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title chore: split Algebra/BigOperators/Group/{Multiset,Finset} [Merged by Bors] - chore: split Algebra/BigOperators/Group/{Multiset,Finset} Jan 13, 2025
@mathlib-bors mathlib-bors bot closed this Jan 13, 2025
@mathlib-bors mathlib-bors bot deleted the split_bigoperators_group branch January 13, 2025 05:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintainer-merge A reviewer has approved the changed; awaiting maintainer approval. ready-to-merge This PR has been sent to bors.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants