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

chore: Small variable cleanup in FieldTheory/RatFunc #4373

Closed
wants to merge 1 commit into from

Conversation

adomani
Copy link
Collaborator

@adomani adomani commented May 26, 2023

Removes some unnecessary introduction of new variables, favouring recycling of pre-existing one. This exploits the new feature of getting Lean to figure out what variables to include/omit automatically.


Open in Gitpod

@adomani adomani added the awaiting-review The author would like community review of the PR label May 26, 2023
@eric-wieser
Copy link
Member

I think the hacks here might be symptomatic of a bigger issue that happened when porting this file, so I'm wary of merging this as is.

@adomani
Copy link
Collaborator Author

adomani commented May 26, 2023

Eric, would a better solution be to add various sections, to isolate the CommRing, IsDomain, Field instances explicitly, rather than relying on Lean to "get it right"?

@eric-wieser
Copy link
Member

Yes, that certainly sounds better; though that might be a post-port job.

@adomani adomani added after-port and removed awaiting-review The author would like community review of the PR labels May 26, 2023
@adomani
Copy link
Collaborator Author

adomani commented May 26, 2023

Ok, I'll leave this be, and mark it as after-port.

@Ruben-VandeVelde Ruben-VandeVelde changed the title chore(FieldTheory/RatFunc): Small variable cleanup chore: Small variable cleanup in FieldTheory/RatFunc May 26, 2023
bors bot pushed a commit to leanprover-community/mathlib that referenced this pull request Jun 1, 2023
…19133)

This hopefully closes [#4513](leanprover-community/mathlib4#4513) (or at least mitigates it) and will settle [!4#4373](leanprover-community/mathlib4#4373) in mathlib3.
bors bot pushed a commit to leanprover-community/mathlib that referenced this pull request Jun 1, 2023
…19133)

This hopefully closes [#4513](leanprover-community/mathlib4#4513) (or at least mitigates it) and will settle [!4#4373](leanprover-community/mathlib4#4373) in mathlib3.




Co-authored-by: Anne Baanen <Vierkantor@users.noreply.github.com>
@bors bors bot closed this in b4fa258 Jun 3, 2023
qawbecrdtey pushed a commit to qawbecrdtey/greedoid-mathlib4 that referenced this pull request Jun 12, 2023
…#4580)

This PR re-ports `FieldTheory/RatFunc`, now that the mathlib3 version uses `section`s, after PR[#19133](leanprover-community/mathlib#19133).

This closes [leanprover-community#4513](leanprover-community#4513) and closes leanprover-community#4373.



Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
@int-y1 int-y1 deleted the adomani_RatFunc_varCleanup branch July 1, 2023 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants