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(NNRat): Rearrange imports #10392

Closed
wants to merge 11 commits into from

Conversation

YaelDillies
Copy link
Collaborator

@YaelDillies YaelDillies commented Feb 9, 2024

The goal is to separate the field material on Rat/NNRat from everything before to make way for NNRat.cast. We achieve this by

  • splitting Data.Rat.NNRat into
    • Data.NNRat.Defs for the foundationl stuff that will be needed in the definition of Field
    • Data.NNRat.Lemmas for the field and big operators material
  • moving the field material from Data.Rat.Order to Data.Rat.Basic
  • proving a few lemmas by rfl rather than coeHom.some_now_unavailable_lemma
  • renaming Data.Rat.NNRat.BigOperators to Data.NNRat.BigOperators

This is a rehash of leanprover-community/mathlib3#18609.

Open in Gitpod

The goal is to separate the field material on `Rat`/`NNRat` from everything before. We achieve this by
* splitting `Data.Rat.NNRat` into
  * `Data.NNRat.Defs` for the foundationl stuff that will be needed in the definition of `Field`
  * `Data.NNRat.Lemmas` for the field and big operators material
* moving the field material from `Data.Rat.Order` to `Data.Rat.Basic`
* proving a few lemmas by `rfl` rather than `coeHom.some_now_unavailable_lemma`
* renaming `Data.Rat.NNRat.BigOperators` to `Data.NNRat.BigOperators`
Mathlib.lean Outdated Show resolved Hide resolved
@YaelDillies YaelDillies added awaiting-review awaiting-CI t-algebra Algebra (groups, rings, fields, etc) t-order Order theory labels Feb 9, 2024
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps this should be called NNRat/Field?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There's more than just the field instance, though?

Copy link
Member

@eric-wieser eric-wieser left a comment

Choose a reason for hiding this comment

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

The Lemmas file feels a bit messy to me (aesthetically I'd prefer to see Field to match Rat, and a separate Lemmas file), but since nothing in practice actually motivates that split I'm fine with a single Lemmas file.

I'll maintainer merge this to let someone else take a look once you've addressed my remaining minor comments.

Copy link
Member

@eric-wieser eric-wieser left a comment

Choose a reason for hiding this comment

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

maintainer merge

Copy link

🚀 Pull request has been placed on the maintainer queue by eric-wieser.

Copy link
Contributor

@Vierkantor Vierkantor left a comment

Choose a reason for hiding this comment

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

bors d+

Mathlib/Data/NNRat/Lemmas.lean Show resolved Hide resolved
@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Feb 12, 2024

✌️ YaelDillies can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@YaelDillies
Copy link
Collaborator Author

bors merge

mathlib-bors bot pushed a commit that referenced this pull request Feb 12, 2024
The goal is to separate the field material on `Rat`/`NNRat` from everything before to make way for `NNRat.cast`. We achieve this by
* splitting `Data.Rat.NNRat` into
  * `Data.NNRat.Defs` for the foundationl stuff that will be needed in the definition of `Field`
  * `Data.NNRat.Lemmas` for the field and big operators material
* moving the field material from `Data.Rat.Order` to `Data.Rat.Basic`
* proving a few lemmas by `rfl` rather than `coeHom.some_now_unavailable_lemma`
* renaming `Data.Rat.NNRat.BigOperators` to `Data.NNRat.BigOperators`
@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Feb 12, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title chore(NNRat): Rearrange imports [Merged by Bors] - chore(NNRat): Rearrange imports Feb 12, 2024
@mathlib-bors mathlib-bors bot closed this Feb 12, 2024
@mathlib-bors mathlib-bors bot deleted the shuffle_nnrat branch February 12, 2024 11:39
dagurtomas pushed a commit that referenced this pull request Mar 22, 2024
The goal is to separate the field material on `Rat`/`NNRat` from everything before to make way for `NNRat.cast`. We achieve this by
* splitting `Data.Rat.NNRat` into
  * `Data.NNRat.Defs` for the foundationl stuff that will be needed in the definition of `Field`
  * `Data.NNRat.Lemmas` for the field and big operators material
* moving the field material from `Data.Rat.Order` to `Data.Rat.Basic`
* proving a few lemmas by `rfl` rather than `coeHom.some_now_unavailable_lemma`
* renaming `Data.Rat.NNRat.BigOperators` to `Data.NNRat.BigOperators`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
delegated t-algebra Algebra (groups, rings, fields, etc) t-order Order theory
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants