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] - chore(data/polynomial): partial move from is_ring_hom to ring_hom #3213

Closed
wants to merge 32 commits into from

Conversation

semorrison
Copy link
Collaborator

@semorrison semorrison commented Jun 28, 2020

This does not attempt to do a complete refactor of polynomial.lean and mv_polynomial.lean to remove use of is_ring_hom, but only to fix eval₂ which we need for the current work on Cayley-Hamilton.

Having struggled with these two files for the last few days, I'm keen to get them cleaned up, so I'll promise to return to this more thoroughly in a later PR.


Blocked by #3193.

@semorrison semorrison added the blocked-by-other-PR This PR depends on another PR which is still in the queue. A bot manages this label via PR comment. label Jun 28, 2020
@semorrison semorrison changed the title chore(data/polynomial): partial move from is_ring_hom to ring_hom chore(data/polynomial): partial move from is_ring_hom to ring_hom Jun 28, 2020
@semorrison semorrison changed the title chore(data/polynomial): partial move from is_ring_hom to ring_hom chore(data/polynomial): partial move from is_ring_hom to ring_hom (deps: 3193) Jun 28, 2020
@jcommelin jcommelin removed the blocked-by-other-PR This PR depends on another PR which is still in the queue. A bot manages this label via PR comment. label Jun 29, 2020
@jcommelin jcommelin changed the title chore(data/polynomial): partial move from is_ring_hom to ring_hom (deps: 3193) chore(data/polynomial): partial move from is_ring_hom to ring_hom Jun 29, 2020
@jcommelin
Copy link
Member

12 conflicts

@bryangingechen bryangingechen added the awaiting-author A reviewer has asked the author a question or requested changes label Jun 29, 2020
@semorrison
Copy link
Collaborator Author

Hopefully the conflicts have been resolved successfully. We'll see if it builds!

@semorrison semorrison 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 Jun 30, 2020
@@ -129,7 +129,12 @@ local attribute [instance] coeff_coe_to_fun
def monomial (s : σ →₀ ℕ) (a : α) : mv_polynomial σ α := single s a

/-- `C a` is the constant polynomial with value `a` -/
def C (a : α) : mv_polynomial σ α := monomial 0 a
def C : α →+* mv_polynomial σ α :=
Copy link
Member

Choose a reason for hiding this comment

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

How about making monomial and add_monoid_hom first?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I agree.

Could I ask though that I defer making significant further changes for a later PR? I definitely want to change a lot more in polynomial.lean and mv_polynomial.lean, but the cayley_hamilton branch can go in for review once this PR is merged, and if we wait for a thorough refactor of polynomials it will start rotting. :-)

Copy link
Member

Choose a reason for hiding this comment

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

Sure!

@jcommelin
Copy link
Member

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 Jun 30, 2020
bors bot pushed a commit that referenced this pull request Jun 30, 2020
)

This does not attempt to do a complete refactor of `polynomial.lean` and `mv_polynomial.lean` to remove use of `is_ring_hom`, but only to fix `eval₂` which we need for the current work on Cayley-Hamilton.

Having struggled with these two files for the last few days, I'm keen to get them cleaned up, so I'll promise to return to this more thoroughly in a later PR.



Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
Co-authored-by: Alex J. Best <alex.j.best@gmail.com>
@bors
Copy link

bors bot commented Jun 30, 2020

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title chore(data/polynomial): partial move from is_ring_hom to ring_hom [Merged by Bors] - chore(data/polynomial): partial move from is_ring_hom to ring_hom Jun 30, 2020
@bors bors bot closed this Jun 30, 2020
@bors bors bot deleted the eval2_ring_hom branch June 30, 2020 06:44
cipher1024 pushed a commit to cipher1024/mathlib that referenced this pull request Mar 15, 2022
…anprover-community#3213)

This does not attempt to do a complete refactor of `polynomial.lean` and `mv_polynomial.lean` to remove use of `is_ring_hom`, but only to fix `eval₂` which we need for the current work on Cayley-Hamilton.

Having struggled with these two files for the last few days, I'm keen to get them cleaned up, so I'll promise to return to this more thoroughly in a later PR.



Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
Co-authored-by: Alex J. Best <alex.j.best@gmail.com>
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