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 (RingTheory/Valuation/Integers) : add lemma one_of_isUnit' #12247

Closed
wants to merge 6 commits into from

Conversation

faenuccio
Copy link
Collaborator

Add lemma one_of_isUnit' , that is a generalization of one_of_isUnit with a slightly weaker assumption.

Co-authored-by: María Inés de Frutos Fernández @mariainesdff


Open in Gitpod

@faenuccio faenuccio added awaiting-review The author would like community review of the PR t-algebra Algebra (groups, rings, fields etc) labels Apr 18, 2024
@faenuccio faenuccio requested a review from kbuzzard April 19, 2024 15:04
@@ -76,6 +76,14 @@ theorem one_of_isUnit {x : O} (hx : IsUnit x) : v (algebraMap O R x) = 1 :=
exact mul_le_mul_left' (hv.2 (u⁻¹ : Units O)) _
#align valuation.integers.one_of_is_unit Valuation.Integers.one_of_isUnit

theorem one_of_isUnit' {x : O} (hx : IsUnit x) (H : ∀ x, v (algebraMap O R x) ≤ 1) :
Copy link
Member

Choose a reason for hiding this comment

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

Am I correct that this does not use (hv : Integers v O)? Since it is not immediate to see what hypotheses are included or not, maybe use this to golf one_of_isUnit or add a docstring or move it out of the Integers namespace instead to make It clearer?

Copy link
Collaborator Author

@faenuccio faenuccio Apr 20, 2024

Choose a reason for hiding this comment

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

You are certainly right, but since this is never needed when you #check it the variable hv does not show up. I have at any rate followed your suggestion and simplified the proof of one_of_isUnit.

@erdOne erdOne 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 20, 2024
@faenuccio faenuccio 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 24, 2024
@faenuccio
Copy link
Collaborator Author

@erdOne Did you find the time to have another look at my modifications following your suggestions?

@erdOne
Copy link
Member

erdOne commented Apr 24, 2024

Oops didn't notice that this is ready for review again. Thanks for the ping.
maintainer merge

Copy link

🚀 Pull request has been placed on the maintainer queue by erdOne.

@fpvandoorn
Copy link
Member

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 Apr 25, 2024
mathlib-bors bot pushed a commit that referenced this pull request Apr 25, 2024
Add lemma `one_of_isUnit'` , that is a generalization of `one_of_isUnit` with a slightly weaker assumption.

Co-authored-by: María Inés de Frutos Fernández @mariainesdff
@faenuccio
Copy link
Collaborator Author

Oops didn't notice that this is ready for review again. Thanks for the ping. maintainer merge

Thanks ;-)

@mathlib-bors
Copy link

mathlib-bors bot commented Apr 25, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title feat (RingTheory/Valuation/Integers) : add lemma one_of_isUnit' [Merged by Bors] - feat (RingTheory/Valuation/Integers) : add lemma one_of_isUnit' Apr 25, 2024
@mathlib-bors mathlib-bors bot closed this Apr 25, 2024
@mathlib-bors mathlib-bors bot deleted the fae_PR_valOneUnit branch April 25, 2024 01:02
apnelson1 pushed a commit that referenced this pull request May 12, 2024
Add lemma `one_of_isUnit'` , that is a generalization of `one_of_isUnit` with a slightly weaker assumption.

Co-authored-by: María Inés de Frutos Fernández @mariainesdff
callesonne pushed a commit that referenced this pull request May 16, 2024
Add lemma `one_of_isUnit'` , that is a generalization of `one_of_isUnit` with a slightly weaker assumption.

Co-authored-by: María Inés de Frutos Fernández @mariainesdff
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintainer-merge ready-to-merge This PR has been sent to bors. t-algebra Algebra (groups, rings, fields etc)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants