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(field_theory/ratfunc): The numerator and denominator of a rational function are coprime #18652

Closed
wants to merge 3 commits into from

Conversation

YaelDillies
Copy link
Collaborator

@YaelDillies YaelDillies commented Mar 25, 2023

Also make more arguments to gcd_ne_zero_of_left/gcd_ne_zero_of_right implicit.

Co-authored-by: Bhavik Mehta bhavikmehta8@gmail.com


Open in Gitpod

@YaelDillies YaelDillies added awaiting-review The author would like community review of the PR t-algebra Algebra (groups, rings, fields etc) labels Mar 25, 2023

lemma gcd_ne_zero_of_left (p q : R) (hp : p ≠ 0) :
gcd_monoid.gcd p q ≠ 0 :=
lemma gcd_ne_zero_of_left (hp : p ≠ 0) : gcd_monoid.gcd p q ≠ 0 :=
Copy link
Member

Choose a reason for hiding this comment

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

Why did you make q implicit here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This matches the two lemmas right under.

Copy link
Member

@eric-wieser eric-wieser Mar 25, 2023

Choose a reason for hiding this comment

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

I'd argue the change should be in the other direction (or declared out of scope and not made at all)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well, I can keep q explicit if you insist. But this lemma is only used inside this file, and never with explicit arguments.

Copy link
Member

Choose a reason for hiding this comment

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

Let's either keep q explicit or not touch these lemmas at all.

Copy link
Member

Choose a reason for hiding this comment

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

I think it's fine to have these implicit.

bors merge

@@ -84,6 +84,9 @@ begin
rw [mul_div_cancel_left _ hz, mul_left_comm, mul_div_cancel_left _ hz]
end

protected lemma mul_div_cancel' (hb : b ≠ 0) (hab : b ∣ a) : b * (a / b) = a :=
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason this is primed but mul_div_cancel_left is not?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This corresponds to mul_div_cancel'.

@semorrison semorrison added the modifies-synchronized-file This PR touches a files that has already been ported to mathlib4, and may need a synchronization PR. label Mar 28, 2023
bors bot pushed a commit that referenced this pull request Jun 15, 2023
…al function are coprime (#18652)

Also make more arguments to `gcd_ne_zero_of_left`/`gcd_ne_zero_of_right` implicit.

Co-authored-by: Bhavik Mehta <bhavikmehta8@gmail.com>
@bors
Copy link

bors bot commented Jun 15, 2023

Pull request successfully merged into master.

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot changed the title feat(field_theory/ratfunc): The numerator and denominator of a rational function are coprime [Merged by Bors] - feat(field_theory/ratfunc): The numerator and denominator of a rational function are coprime Jun 15, 2023
@bors bors bot closed this Jun 15, 2023
@bors bors bot deleted the ratfunc_coprime_num_denom branch June 15, 2023 17:53
YaelDillies added a commit to leanprover-community/mathlib4 that referenced this pull request Jun 16, 2023
bors bot pushed a commit to leanprover-community/mathlib4 that referenced this pull request Jun 19, 2023
semorrison pushed a commit to leanprover-community/mathlib4 that referenced this pull request Jun 19, 2023
alexkeizer pushed a commit to leanprover-community/mathlib4 that referenced this pull request Jun 22, 2023
semorrison pushed a commit to leanprover-community/mathlib4 that referenced this pull request Jun 23, 2023
semorrison pushed a commit to leanprover-community/mathlib4 that referenced this pull request Jun 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-review The author would like community review of the PR modifies-synchronized-file This PR touches a files that has already been ported to mathlib4, and may need a synchronization PR. t-algebra Algebra (groups, rings, fields etc)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants