Skip to content

[Merged by Bors] - chore(NNRat): Rearrange imports#10392

Closed
YaelDillies wants to merge 11 commits intomasterfrom
shuffle_nnrat
Closed

[Merged by Bors] - chore(NNRat): Rearrange imports#10392
YaelDillies wants to merge 11 commits intomasterfrom
shuffle_nnrat

Conversation

@YaelDillies
Copy link
Copy Markdown
Contributor

@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`
@YaelDillies YaelDillies added awaiting-review awaiting-CI This PR does not pass CI yet. This label is automatically removed once it does. t-algebra Algebra (groups, rings, fields, etc) t-order Order theory labels Feb 9, 2024
Copy link
Copy Markdown
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
Copy Markdown
Contributor 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?

@github-actions github-actions bot removed the awaiting-CI This PR does not pass CI yet. This label is automatically removed once it does. label Feb 10, 2024
Copy link
Copy Markdown
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
Copy Markdown
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

@github-actions
Copy link
Copy Markdown

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

Copy link
Copy Markdown
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-bors
Copy link
Copy Markdown
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.

@ghost ghost added delegated This pull request has been delegated to the PR author (or occasionally another non-maintainer). and removed awaiting-review labels Feb 12, 2024
@YaelDillies
Copy link
Copy Markdown
Contributor 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
Copy Markdown
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 This pull request has been delegated to the PR author (or occasionally another non-maintainer). t-algebra Algebra (groups, rings, fields, etc) t-order Order theory

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants