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] - refactor(algebra/{group,group_with_zero/basic): Delete lemmas generalized to division monoids #14042

Closed
wants to merge 21 commits into from

Conversation

YaelDillies
Copy link
Collaborator

@YaelDillies YaelDillies commented May 9, 2022

Delete the group and group_with_zero lemmas which have been made one-liners in #14000.

Lemmas are renamed because

  • one of the group or group_with_zero name has to go
  • the new API should have a consistent naming convention

Lemma renames

group lemma, group_with_zero lemma → new division_monoid/division_comm_monoid lemma

  • -, inv_eq_of_mul_left_eq_oneinv_eq_of_mul_eq_one_left
  • inv_eq_of_mul_eq_one, inv_eq_of_mul_right_eq_oneinv_eq_of_mul_eq_one_right
  • eq_inv_of_mul_eq_one, eq_inv_of_mul_left_eq_oneeq_inv_of_mul_eq_one_left
  • -, eq_inv_of_mul_right_eq_oneeq_inv_of_mul_eq_one_right
  • -, eq_one_div_of_mul_eq_oneeq_one_div_of_mul_eq_one_right
  • inv_div', inv_divinv_div
  • div_one', div_onediv_one
  • eq_of_div_eq_one', eq_of_div_eq_oneeq_of_div_eq_one
  • div_eq_inv_mul', div_eq_inv_muldiv_eq_inv_mul
  • div_right_comm', div_right_commdiv_right_comm
  • div_mul_eq_mul_div', div_mul_eq_mul_divdiv_mul_eq_mul_div
  • div_mul_comm, div_mul_comm'div_mul_comm
  • mul_comm_div', div_mul_eq_mul_div_commmul_comm_div
  • -, mul_div_commmul_div_left_comm
  • mul_inv, mul_inv₀mul_inv
  • inv_eq_one, inv_eq_one₀inv_eq_one
  • one_eq_inv, one_eq_inv₀one_eq_inv
  • div_div_div_comm, div_div_div_comm₀div_div_div_comm
  • div_mul_div_comm, div_mul_div_comm₀div_mul_div_comm
  • mul_div_comm', - → mul_div_mul_comm
  • -, div_div_div_div_eqdiv_div_div_eq
  • one_inv, inv_oneinv_one
  • div_div, div_div_eq_div_muldiv_div
  • mul_div_left_comm, mul_div_commmul_div_left_comm
  • div_div_assoc_swap, div_div_eq_mul_divdiv_div_eq_mul_div

Other changes

  • Arguments order
  • Arguments implicitness

I found that splitting the refactor in this unusual way allowed me to have a cleaner diff and be systematically sure that I wasn't missing lemmas. The only problem is that I have to unsimp the old lemmas.

Here's what to look out for if you want to check for yourself:

  • All lemma renames are either because of the group/group_with_zero disparity or naming consistency within the new API.
  • simp and field_simp are copied correctly.
  • Every one-lined group/group_with_zero lemma has been deleted.

Open in Gitpod

@YaelDillies YaelDillies added awaiting-review The author would like community review of the PR awaiting-CI The author would like to see what CI has to say before doing more work. labels May 9, 2022
Copy link
Collaborator

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

Impressive work! Everything looks sensible, so let's merge this when it builds.

bors d+

@bors
Copy link

bors bot commented May 9, 2022

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

@Vierkantor Vierkantor added ready-for-bors and removed awaiting-review The author would like community review of the PR labels May 9, 2022
@leanprover-community-bot-assistant leanprover-community-bot-assistant added the delegated The PR author may merge after reviewing final suggestions. label May 9, 2022
@YaelDillies
Copy link
Collaborator Author

bors merge

@bors
Copy link

bors bot commented May 10, 2022

👎 Rejected by label

@YaelDillies YaelDillies removed the awaiting-CI The author would like to see what CI has to say before doing more work. label May 10, 2022
@YaelDillies
Copy link
Collaborator Author

(We're just waiting for lint and it was already working before the last master merge)

bors merge

bors bot pushed a commit that referenced this pull request May 10, 2022
…ized to division monoids (#14042)

Delete the `group` and `group_with_zero` lemmas which have been made one-liners in #14000.

Lemmas are renamed because
* one of the `group` or `group_with_zero` name has to go
* the new API should have a consistent naming convention

Lemma renames
@bors
Copy link

bors bot commented May 10, 2022

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title refactor(algebra/{group,group_with_zero/basic): Delete lemmas generalized to division monoids [Merged by Bors] - refactor(algebra/{group,group_with_zero/basic): Delete lemmas generalized to division monoids May 10, 2022
@bors bors bot closed this May 10, 2022
@bors bors bot deleted the division_monoid_kill_group_lemmas branch May 10, 2022 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
delegated The PR author may merge after reviewing final suggestions. ready-for-bors
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants