Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Merged by Bors] - chore: reduce imports in ZMod.Basic #12478

Closed
wants to merge 6 commits into from

Conversation

Ruben-VandeVelde
Copy link
Collaborator

@Ruben-VandeVelde Ruben-VandeVelde commented Apr 28, 2024

  • While at it, document the lemmas that were moved, and two similar lemmas in ZMod.Basic.

  • The extra import Mathlib.RingTheory.ZMod import in Mathlib/Data/ZMod/Quotient.lean and Mathlib/NumberTheory/Padics/RingHoms.lean is probably not too bad (shake didn't speak yet, so perhaps that is even superfluous).

Open in Gitpod

@grunweg
Copy link
Collaborator

grunweg commented Apr 28, 2024

Commenting to avoid duplicate work: I'm trying to pick this up.

@grunweg
Copy link
Collaborator

grunweg commented Apr 28, 2024

This is almost ready: there is one build error to fix in Data/Zmod/Quotient.

@grunweg grunweg added awaiting-review The author would like community review of the PR and removed please-adopt labels Apr 28, 2024
@grunweg
Copy link
Collaborator

grunweg commented Apr 28, 2024

@Ruben-VandeVelde I'd declare this done (modulo further build errors, but I cannot imagine those). Feel free to take a look.

Copy link
Collaborator Author

@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.

The parts I didn't do look good :)

Mathlib/Data/ZMod/Module.lean Outdated Show resolved Hide resolved
Mathlib/RingTheory/ZMod.lean Outdated Show resolved Hide resolved
import Mathlib.RingTheory.Ideal.Operations
import Mathlib.Data.Fintype.Units
import Mathlib.Data.Nat.Parity
import Mathlib.Algebra.Ring.Prod
import Mathlib.Tactic.FinCases
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this can be removed, #12487 confirms this.

Suggested change
import Mathlib.Tactic.FinCases

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Presumably because it's imported transitively, because this file does use fin_cases

Copy link
Contributor

@Vierkantor Vierkantor left a comment

Choose a reason for hiding this comment

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

I also complained about the fact that shake doesn't warn about transitive imports, but apparently that's supposed to be a feature. So I'll let you decide whether it's best to keep or remove.

Looks good to me otherwise, thanks!

bors d+

@mathlib-bors
Copy link

mathlib-bors bot commented Apr 29, 2024

✌️ Ruben-VandeVelde can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added delegated and removed awaiting-review The author would like community review of the PR labels Apr 29, 2024
@Ruben-VandeVelde
Copy link
Collaborator Author

bors merge

mathlib-bors bot pushed a commit that referenced this pull request May 5, 2024
- While at it, document the lemmas that were moved, and two similar lemmas in `ZMod.Basic`.



Co-authored-by: Michael Rothgang <rothgami@math.hu-berlin.de>
@mathlib-bors
Copy link

mathlib-bors bot commented May 5, 2024

Build failed:

@grunweg
Copy link
Collaborator

grunweg commented May 5, 2024

@Ruben-VandeVelde This builds again :-)

@Ruben-VandeVelde
Copy link
Collaborator Author

bors merge

mathlib-bors bot pushed a commit that referenced this pull request May 5, 2024
- While at it, document the lemmas that were moved, and two similar lemmas in `ZMod.Basic`.



Co-authored-by: Michael Rothgang <rothgami@math.hu-berlin.de>
@mathlib-bors
Copy link

mathlib-bors bot commented May 5, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title chore: reduce imports in ZMod.Basic [Merged by Bors] - chore: reduce imports in ZMod.Basic May 5, 2024
@mathlib-bors mathlib-bors bot closed this May 5, 2024
@mathlib-bors mathlib-bors bot deleted the zmod-basic-trim branch May 5, 2024 17:58
apnelson1 pushed a commit that referenced this pull request May 12, 2024
- While at it, document the lemmas that were moved, and two similar lemmas in `ZMod.Basic`.



Co-authored-by: Michael Rothgang <rothgami@math.hu-berlin.de>
callesonne pushed a commit that referenced this pull request May 16, 2024
- While at it, document the lemmas that were moved, and two similar lemmas in `ZMod.Basic`.



Co-authored-by: Michael Rothgang <rothgami@math.hu-berlin.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants