-
Notifications
You must be signed in to change notification settings - Fork 894
chore: remove extra monic hypotheses from divByMonic and modByMonic lemmas #31817
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
base: master
Are you sure you want to change the base?
Conversation
PR summary a73de4ba4bImport changes for modified filesNo significant changes to the import graph Import changes for all files
Declarations diffNo 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 No changes to technical debt.You can run this locally as
|
Mathlib/Algebra/Polynomial/Div.lean
Outdated
| unfold modByMonic divByMonic divModByMonicAux | ||
| dsimp |
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.
given that github has mangled the diff here anyway (i.e. isn't just saying "you added two lines" it's saying "you deleted a proof and replaced it with what looks to me like a different proof but actually it's the same proof indented two spaces more" you may as well golf this if you can -- can all the unfolds be put into the dsimp and rw?
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.
I've golfed it a bit, but the unfold and rw still can't be combined to one line (dsimp doesn't work either)
|
Nice -- this is clearly better. |
artie2000
left a comment
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.
Doesn't look like you missed any lemmas!
| refine ih.trans ?_ | ||
| rw [divByMonic, dif_pos hq, dif_pos hq, dif_pos h, mul_add, sub_add_eq_sub_sub] | ||
| else by | ||
| unfold modByMonic divByMonic divModByMonicAux |
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.
Move divByMonic_eq_of_not_monic and modByMonic_eq_of_not_monic earlier so the non-monic branches you add can be closed in one line?
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.
But it is already one line (here), and it already uses divByMonic_eq_of_not_monic and modByMonic_eq_of_not_monic, and what you linked to isn't the non-monic branch, but rather the branch that says "division is done here and you can go home and have dinner"
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.
my bad!
|
LGTM |
jcommelin
left a comment
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.
Thanks 🎉
bors merge
|
Build failed (retrying...): |
|
Build failed (retrying...): |
|
Build failed (retrying...): |
|
I think this is failing: https://github.com/leanprover-community/mathlib4/actions/runs/19569815604/job/56040389958#step:37:20 |
|
✌️ kckennylau can now approve this pull request. To approve and merge a pull request, simply reply with |
|
Canceled. |
Zulip discussion