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] - feat(algebra/order/sub): An add_group has ordered subtraction #10225

Closed
wants to merge 6 commits into from

Conversation

YaelDillies
Copy link
Collaborator

This wraps up sub_le_iff_le_add in an has_ordered_sub instance.


Open in Gitpod

@YaelDillies YaelDillies added the awaiting-review The author would like community review of the PR label Nov 8, 2021
Copy link
Member

@eric-wieser eric-wieser left a comment

Choose a reason for hiding this comment

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

Does it make more sense to do this in the other direction, and have group import sub?

@YaelDillies
Copy link
Collaborator Author

I think so too! because then we can also deduplicate the two lemmas: we get rid of sub_le_iff_le_add and rename tsub_le_iff_right to sub_le_iff_right or sub_le_iff_le_add.
What do you think?

@eric-wieser
Copy link
Member

I'd leave the renaming for another PR, but I think it probably makes sense.

src/algebra/order/sub.lean Outdated Show resolved Hide resolved
@fpvandoorn
Copy link
Member

I think this instance is indeed good to add.

I'm not sure if I'm in favor of renaming tsub_le_iff_right. It depends a bit on how much lemmas will be general enough to apply in both cases. Many lemmas that hold for group require canonically_ordered_add_monoid in the order.sub file, so we will have to keep them as duplicates. And many other lemmas need extra assumptions in the has_ordered_sub case. So if there are only a couple of lemmas that are the same for has_ordered_sub / add_group, then I would keep both names (with sub and with tsub). We can of course still make the sub lemmas aliases of the tsub lemmas.
If there are a lot (which I don't expect), we could reconsider.

Copy link
Member

@jcommelin jcommelin left a comment

Choose a reason for hiding this comment

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

Thanks 🎉

bors merge

@github-actions github-actions bot added ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) and removed awaiting-review The author would like community review of the PR labels Nov 11, 2021
bors bot pushed a commit that referenced this pull request Nov 11, 2021
This wraps up `sub_le_iff_le_add` in an `has_ordered_sub` instance.
@bors
Copy link

bors bot commented Nov 11, 2021

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(algebra/order/sub): An add_group has ordered subtraction [Merged by Bors] - feat(algebra/order/sub): An add_group has ordered subtraction Nov 11, 2021
@bors bors bot closed this Nov 11, 2021
@bors bors bot deleted the to_has_ordered_sub branch November 11, 2021 21:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants