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(ring_theory/polynomial/opposites + data/{polynomial/basic + finsupp/basic}): move op_ring_equiv to a new file + lemmas #13162

Closed
wants to merge 12 commits into from

Conversation

adomani
Copy link
Collaborator

@adomani adomani commented Apr 4, 2022

This PR moves the isomorphism R[X]ᵐᵒᵖ ≃+* Rᵐᵒᵖ[X] to a new file ring_theory.polynomial.opposites.

I also proved some basic lemmas about the equivalence.

Zulip discussion


Open in Gitpod

@adomani adomani added the awaiting-review The author would like community review of the PR label Apr 4, 2022
adomani and others added 3 commits April 4, 2022 16:18
@adomani adomani changed the title feat(ring_theory/polynomial/opposites + data/polynomial/basic): move op_ring_equiv to a new file + lemmas feat(ring_theory/polynomial/opposites + data/{polynomial/basic + finsupp/basic}): move op_ring_equiv to a new file + lemmas Apr 4, 2022
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.

A bunch of parens can be removed.

Can you add the lemmas for (op_ring_equiv R).symm X and the like as well?

src/ring_theory/polynomial/opposites.lean Outdated Show resolved Hide resolved
src/ring_theory/polynomial/opposites.lean Outdated Show resolved Hide resolved
src/ring_theory/polynomial/opposites.lean Outdated Show resolved Hide resolved
src/ring_theory/polynomial/opposites.lean Outdated Show resolved Hide resolved
adomani and others added 3 commits April 4, 2022 18:19
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
@adomani
Copy link
Collaborator Author

adomani commented Apr 4, 2022

I'll add the remaining lemmas, though probably not today!

@semorrison semorrison 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 Apr 5, 2022
@adomani adomani 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 Apr 5, 2022
@adomani
Copy link
Collaborator Author

adomani commented Apr 5, 2022

Eric, I added the lemmas. Let me know if this is not what you had in mind!

I also added a couple of doc-module strings to guide through the structure of the (very short) file.

@eric-wieser
Copy link
Member

Would you mind merging master?


/-! Lemmas to get started, using `(op_ring_equiv R).symm` on the various expressions of
`finsupp.single`: `monomial`, `C a`, `X`, `C a * X ^ n`. -/
@[simp] lemma op_ring_equiv_symm_op_monomial (n : ℕ) (r : Rᵐᵒᵖ) :
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
@[simp] lemma op_ring_equiv_symm_op_monomial (n : ℕ) (r : Rᵐᵒᵖ) :
@[simp] lemma op_ring_equiv_symm_monomial (n : ℕ) (r : Rᵐᵒᵖ) :

This lemma and a few below have an op in the name that shouldn't be there

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks! I'll update the names and will also merge master: the change you mentioned made R implicit and I am in the process of fixing it!

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure I made R implicit consciously - feel free to keep it explicit

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually, you have not: it was some weird issue probably due to old oleans being used instead of the new ones.

Copy link
Collaborator Author

@adomani adomani Apr 5, 2022

Choose a reason for hiding this comment

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

On the topic of explicit vs implicit R: at the moment, R could be made implicit, since there are enough type annotation that it is redundant. However, I feel that using or_ring_equiv is a little bit further below what I would expect to see when actually using an Rᵐᵒᵖ[X]. So, for "regular" usage, it should not matter, since the simp lemmas in this file make or_ring_equiv disappear.

However, if you really want to have control, you may prefer to take the opposite of a subring, rather than the "obvious one" and then having the explicit R could be helpful.

@adomani
Copy link
Collaborator Author

adomani commented Apr 5, 2022

I merged master, re-squeezed the first proof for the monomial lemma and removed the op. Everything else seems to have survived unscathed your refactor!

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

@leanprover-community-bot-assistant leanprover-community-bot-assistant 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 Apr 9, 2022
bors bot pushed a commit that referenced this pull request Apr 9, 2022
…upp/basic}): move `op_ring_equiv` to a new file + lemmas (#13162)

This PR moves the isomorphism `R[X]ᵐᵒᵖ ≃+* Rᵐᵒᵖ[X]` to a new file `ring_theory.polynomial.opposites`.

I also proved some basic lemmas about the equivalence.

[Zulip discussion](https://leanprover.zulipchat.com/#narrow/stream/113489-new-members)
@bors
Copy link

bors bot commented Apr 9, 2022

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Apr 9, 2022
…upp/basic}): move `op_ring_equiv` to a new file + lemmas (#13162)

This PR moves the isomorphism `R[X]ᵐᵒᵖ ≃+* Rᵐᵒᵖ[X]` to a new file `ring_theory.polynomial.opposites`.

I also proved some basic lemmas about the equivalence.

[Zulip discussion](https://leanprover.zulipchat.com/#narrow/stream/113489-new-members)
@bors
Copy link

bors bot commented Apr 9, 2022

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Apr 9, 2022
…upp/basic}): move `op_ring_equiv` to a new file + lemmas (#13162)

This PR moves the isomorphism `R[X]ᵐᵒᵖ ≃+* Rᵐᵒᵖ[X]` to a new file `ring_theory.polynomial.opposites`.

I also proved some basic lemmas about the equivalence.

[Zulip discussion](https://leanprover.zulipchat.com/#narrow/stream/113489-new-members)
@bors
Copy link

bors bot commented Apr 9, 2022

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Apr 10, 2022
…upp/basic}): move `op_ring_equiv` to a new file + lemmas (#13162)

This PR moves the isomorphism `R[X]ᵐᵒᵖ ≃+* Rᵐᵒᵖ[X]` to a new file `ring_theory.polynomial.opposites`.

I also proved some basic lemmas about the equivalence.

[Zulip discussion](https://leanprover.zulipchat.com/#narrow/stream/113489-new-members)
@bors
Copy link

bors bot commented Apr 10, 2022

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Apr 10, 2022
…upp/basic}): move `op_ring_equiv` to a new file + lemmas (#13162)

This PR moves the isomorphism `R[X]ᵐᵒᵖ ≃+* Rᵐᵒᵖ[X]` to a new file `ring_theory.polynomial.opposites`.

I also proved some basic lemmas about the equivalence.

[Zulip discussion](https://leanprover.zulipchat.com/#narrow/stream/113489-new-members)
@bors
Copy link

bors bot commented Apr 10, 2022

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(ring_theory/polynomial/opposites + data/{polynomial/basic + finsupp/basic}): move op_ring_equiv to a new file + lemmas [Merged by Bors] - feat(ring_theory/polynomial/opposites + data/{polynomial/basic + finsupp/basic}): move op_ring_equiv to a new file + lemmas Apr 10, 2022
@bors bors bot closed this Apr 10, 2022
@bors bors bot deleted the adomani_op_C branch April 10, 2022 10:48
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

5 participants