-
Notifications
You must be signed in to change notification settings - Fork 298
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(ring_theory/ideals): Move the definition of ideals out of algebra/module #3692
Conversation
…ra/module Also adds a very minimal docstring
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ring theory does seem like a more appropriate place. Is there a reason for this change apart from neatness (e.g. better compilation times)?
Co-authored-by: Vierkantor <Vierkantor@users.noreply.github.com>
Neatness was the main motivation - it makes it easier to reason about what would need doing in #3635. As a follow-up, I wonder if (again for the sake of neatness), the following renames should happen in some future PR:
|
Could you fix the broken build? It looks like the implicit variables are being declared in a different order for |
…ity/mathlib into move-ideal
Unfortunately, there are two old ways here. To avoid having to change every caller of these theorems, I've tried to leave all the lemmas with the same argument order they have before. At some point, someone should probably make them use the same argument order, but that feels out of scope for this PR. |
Thanks 🎉 bors r+ |
Pull request successfully merged into master. Build succeeded: |
Neatness was the main motivation - it makes it easier to reason about what would need doing in #3635.
It also results in somewhere sensible for the docs about ideals. Also adds a very minimal docstring to
ring_theory/ideals.lean
.