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: Port RingTheory/RingInvo #1276

Closed
wants to merge 18 commits into from

Conversation

aricursion
Copy link
Collaborator

Everything compiles properly, not done with style/removing unnecisary lemmas

@aricursion aricursion added question mathlib-port This is a port of a theory file from mathlib. help-wanted The author needs attention to resolve issues labels Dec 31, 2022
@semorrison semorrison added the merge-conflict The PR has a merge conflict with master, and needs manual merging. label Jan 2, 2023
@semorrison semorrison removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. label Jan 3, 2023
Mathlib/RingTheory/RingInvo.lean Show resolved Hide resolved
Comment on lines 74 to 77
@[simp]
theorem map_eq_zero_iff (f : RingInvo R) {x : R} : f x = 0 ↔ x = 0 :=
f.toRingEquiv.map_eq_zero_iff
#align ring_invo.map_eq_zero_iff RingInvo.map_eq_zero_iff
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
@[simp]
theorem map_eq_zero_iff (f : RingInvo R) {x : R} : f x = 0 ↔ x = 0 :=
f.toRingEquiv.map_eq_zero_iff
#align ring_invo.map_eq_zero_iff RingInvo.map_eq_zero_iff
/- Porting note `simp` attribute removed as suggested by linter:
simp can prove this:
by simp only [AddEquivClass.map_eq_zero_iff]
#noalign ring_invo.map_eq_zero_iff

Again I'm not sure how to handle the linter error here correcty. If one replaces f x by f.toRingEquiv as above, the linter says it is no needed anymore.

@aricursion aricursion added the WIP Work in progress label Jan 14, 2023
@LukasMias LukasMias added merge-conflict The PR has a merge conflict with master, and needs manual merging. mathlib3-pair This PR is a forward-port of a mathlib3 PR or part of one, either under review or recently merged and removed merge-conflict The PR has a merge conflict with master, and needs manual merging. labels Jan 15, 2023
@LukasMias
Copy link
Collaborator

LukasMias commented Jan 15, 2023

The whole RingInvoClass business needs to be backported before we can merge this PR. The mathlib3 PR for this is found here. I'm removing the WIP, help, and question tags because I think this PR is finished up to these changes.

@LukasMias LukasMias removed help-wanted The author needs attention to resolve issues question WIP Work in progress labels Jan 15, 2023
@LukasMias
Copy link
Collaborator

The whole RingInvoClass business needs to be backported before we can merge this PR. The mathlib3 PR for this is found here. I'm removing the WIP, help, and question tags because I think this PR is finished up to these changes.

The mathlib3 PR is merged now! I will wait to see how mathport translates that file once that happens, and then we can finish things up here :)

@LukasMias LukasMias added awaiting-review The author would like community review of the PR and removed mathlib3-pair This PR is a forward-port of a mathlib3 PR or part of one, either under review or recently merged labels Jan 18, 2023
@semorrison semorrison added the merge-conflict The PR has a merge conflict with master, and needs manual merging. label Jan 18, 2023
@aricursion
Copy link
Collaborator Author

All that's left is to decide whether we want instance hasCoeToRingEquiv I'm not sure what the answer here is

@aricursion aricursion removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. label Jan 18, 2023
@jcommelin jcommelin added awaiting-author A reviewer has asked the author a question or requested changes and removed awaiting-review The author would like community review of the PR labels Jan 19, 2023
@LukasMias LukasMias added awaiting-review The author would like community review of the PR and removed awaiting-author A reviewer has asked the author a question or requested changes labels Jan 26, 2023
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

Mathlib/RingTheory/RingInvo.lean Show resolved Hide resolved
@semorrison semorrison added ready-to-merge This PR has been sent to bors. and removed awaiting-review The author would like community review of the PR labels Jan 27, 2023
bors bot pushed a commit that referenced this pull request Jan 27, 2023
Everything compiles properly, not done with style/removing unnecisary lemmas

Co-authored-by: Heather Macbeth <25316162+hrmacbeth@users.noreply.github.com>
Co-authored-by: Ruben Van de Velde <65514131+Ruben-VandeVelde@users.noreply.github.com>
Co-authored-by: qawbecrdtey <qawbecrdtey@kaist.ac.kr>
Co-authored-by: Lukas Miaskiwskyi <lukas.mias@gmail.com>
Co-authored-by: z <32079362+zaxioms@users.noreply.github.com>
@bors
Copy link

bors bot commented Jan 27, 2023

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat: Port RingTheory/RingInvo [Merged by Bors] - feat: Port RingTheory/RingInvo Jan 27, 2023
@bors bors bot closed this Jan 27, 2023
@bors bors bot deleted the port/RingTheory.RingInvo branch January 27, 2023 06:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mathlib-port This is a port of a theory file from mathlib. ready-to-merge This PR has been sent to bors.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants