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 Algebra.Hom.NonUnitalAlg #2414

Closed
wants to merge 9 commits into from

Conversation

jcommelin
Copy link
Member


Open in Gitpod

Mathbin -> Mathlib

fix certain import statements

move "by" to end of line

add import to Mathlib.lean
@jcommelin jcommelin added the mathlib-port This is a port of a theory file from mathlib. label Feb 21, 2023
@jcommelin jcommelin added the help-wanted The author needs attention to resolve issues label Feb 21, 2023
Copy link
Contributor

@ocfnash ocfnash left a comment

Choose a reason for hiding this comment

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

There are currently no build errors but six linter errors, all from the simpNF linter. These six come in pairs associated to the definitions of NonUnitalAlgHom.fst, NonUnitalAlgHom.snd, NonUnitalAlgHom.prod.

The errors can be solved by writing three simp lemmas manually instead of using simps (as shown in suggestions). It would be good to know if this is a limitation of simps or a case of pilot error.

In particular it would be good to understand why simps is generating lemmas of the form:

NonUnitalAlgHom.fst_toDistribMulActionHom_toMulActionHom_toFun

Mathlib/Algebra/Hom/NonUnitalAlg.lean Show resolved Hide resolved
Mathlib/Algebra/Hom/NonUnitalAlg.lean Show resolved Hide resolved
Mathlib/Algebra/Hom/NonUnitalAlg.lean Show resolved Hide resolved
Mathlib/Algebra/Hom/NonUnitalAlg.lean Show resolved Hide resolved
Mathlib/Algebra/Hom/NonUnitalAlg.lean Show resolved Hide resolved
Mathlib/Algebra/Hom/NonUnitalAlg.lean Show resolved Hide resolved
Mathlib/Algebra/Hom/NonUnitalAlg.lean Show resolved Hide resolved
@fpvandoorn
Copy link
Member

fpvandoorn commented Feb 28, 2023

There are currently no build errors but six linter errors, all from the simpNF linter. These six come in pairs associated to the definitions of NonUnitalAlgHom.fst, NonUnitalAlgHom.snd, NonUnitalAlgHom.prod.

The errors can be solved by writing three simp lemmas manually instead of using simps (as shown in suggestions). It would be good to know if this is a limitation of simps or a case of pilot error.

In particular it would be good to understand why simps is generating lemmas of the form:

NonUnitalAlgHom.fst_toDistribMulActionHom_toMulActionHom_toFun

This is just a misconfiguration.
The initialize_simps_projections needs a -toDistribMulActionHom argument. Also, you probably want to define a custom NonUnitalAlgHom.Simps.apply. Mimic the code of MonoidHom or something. Or merge #2045, after which no custom configuration is needed at all.

@jcommelin jcommelin added awaiting-review The author would like community review of the PR and removed help-wanted The author needs attention to resolve issues labels Feb 28, 2023
And other very minor tweaks.
@ocfnash
Copy link
Contributor

ocfnash commented Feb 28, 2023

Thanks @fpvandoorn

What do we need to do to get #2045 / #2042 merged? Are they just waiting for final review?

@jcommelin
Copy link
Member Author

Thanks 🎉

bors merge

@github-actions github-actions bot 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 Feb 28, 2023
bors bot pushed a commit that referenced this pull request Feb 28, 2023
Co-authored-by: Lukas Miaskiwskyi <lukas.mias@gmail.com>
Co-authored-by: Oliver Nash <github@olivernash.org>
#align non_unital_alg_hom.coe_mk NonUnitalAlgHom.coe_mk

@[simp]
theorem mk_coe (f : A →ₙₐ[R] B) (h₁ h₂ h₃ h₄) : (⟨⟨⟨f, h₁⟩, h₂, h₃⟩, h₄⟩ : A →ₙₐ[R] B) = f := by
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
theorem mk_coe (f : A →ₙₐ[R] B) (h₁ h₂ h₃ h₄) : (⟨⟨⟨f, h₁⟩, h₂, h₃⟩, h₄⟩ : A →ₙₐ[R] B) = f := by
theorem mk_coe (f : A →ₙₐ[R] B) (h₁ h₂ h₃ h₄) : (⟨⟨⟨f, h₁⟩, h₂, h₃⟩, h₄⟩ : A →ₙₐ[R] B) = f :=

and similarly elsewhere

@bors
Copy link

bors bot commented Feb 28, 2023

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat: port Algebra.Hom.NonUnitalAlg [Merged by Bors] - feat: port Algebra.Hom.NonUnitalAlg Feb 28, 2023
@bors bors bot closed this Feb 28, 2023
@bors bors bot deleted the port/Algebra.Hom.NonUnitalAlg branch February 28, 2023 17:07
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

5 participants