Skip to content

[Merged by Bors] - refactor(Data/Rat/NNRat): move module and algebra instances#9951

Closed
eric-wieser wants to merge 7 commits intomasterfrom
eric-wieser/move-nnrat-module
Closed

[Merged by Bors] - refactor(Data/Rat/NNRat): move module and algebra instances#9951
eric-wieser wants to merge 7 commits intomasterfrom
eric-wieser/move-nnrat-module

Conversation

@eric-wieser
Copy link
Copy Markdown
Member

@eric-wieser eric-wieser commented Jan 24, 2024

As with #9950, this is motivated by:

  • Getting NNRat closed to norm_num
  • Being able to put an nnrat_cast field in DivisionSemirings

This brings down the number of dependencies of NNRat by around 600.


Open in Gitpod

The graph is still a monster:

nnrat

We already have the instances for `ℕ`, `ℤ`, and `ℚ` in this file, so adding `NNRat` doesn't feel that out of place.
@eric-wieser eric-wieser added awaiting-author A reviewer has asked the author a question or requested changes. awaiting-CI This PR does not pass CI yet. This label is automatically removed once it does. labels Jan 24, 2024
@ghost ghost added blocked-by-other-PR This PR depends on another PR (this label is automatically managed by a bot) and removed blocked-by-other-PR This PR depends on another PR (this label is automatically managed by a bot) labels Jan 24, 2024
Authors: Yaël Dillies, Bhavik Mehta
-/
import Mathlib.Algebra.Algebra.Basic
import Mathlib.Algebra.Function.Indicator
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Removing this would remove another 30 or so dependencies, but I'm declaring that a problem for a follow-up. I think further reduction probably entails splitting this file.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note that I already reduced the dependencies of Algebra.Function.Indicator by quite a lot because of this.

@eric-wieser eric-wieser added awaiting-review and removed awaiting-author A reviewer has asked the author a question or requested changes. labels Jan 24, 2024
@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 Jan 25, 2024
@YaelDillies
Copy link
Copy Markdown
Contributor

I just noticed that Algebra.Order.Archimedean depends on LinearAlgebra.Basic through NNRat 😱

maintainer merge

@github-actions
Copy link
Copy Markdown

github-actions bot commented Feb 1, 2024

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

@eric-wieser
Copy link
Copy Markdown
Member Author

I just noticed that Algebra.Order.Archimedean depends on LinearAlgebra.Basic through NNRat 😱

Only before this PR though, right?

Copy link
Copy Markdown
Member

@jcommelin jcommelin left a comment

Choose a reason for hiding this comment

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

Thanks 🎉

bors merge

@ghost ghost added ready-to-merge This PR has been sent to bors. and removed awaiting-review labels Feb 1, 2024
@YaelDillies
Copy link
Copy Markdown
Contributor

I just noticed that Algebra.Order.Archimedean depends on LinearAlgebra.Basic through NNRat 😱

Only before this PR though, right?

Actually, this spurious dependency was introduced in #9950.

mathlib-bors bot pushed a commit that referenced this pull request Feb 1, 2024
As with #9950, this is motivated by:
* Getting `NNRat` closed to `norm_num`
* Being able to put an `nnrat_cast` field in `DivisionSemiring`s

This brings down the number of dependencies of `NNRat` by around 600.
@mathlib-bors
Copy link
Copy Markdown
Contributor

mathlib-bors bot commented Feb 1, 2024

Build failed (retrying...):

mathlib-bors bot pushed a commit that referenced this pull request Feb 1, 2024
As with #9950, this is motivated by:
* Getting `NNRat` closed to `norm_num`
* Being able to put an `nnrat_cast` field in `DivisionSemiring`s

This brings down the number of dependencies of `NNRat` by around 600.
@mathlib-bors
Copy link
Copy Markdown
Contributor

mathlib-bors bot commented Feb 1, 2024

Build failed (retrying...):

  • Build

mathlib-bors bot pushed a commit that referenced this pull request Feb 1, 2024
As with #9950, this is motivated by:
* Getting `NNRat` closed to `norm_num`
* Being able to put an `nnrat_cast` field in `DivisionSemiring`s

This brings down the number of dependencies of `NNRat` by around 600.
@mathlib-bors
Copy link
Copy Markdown
Contributor

mathlib-bors bot commented Feb 1, 2024

Build failed:

@eric-wieser
Copy link
Copy Markdown
Member Author

@ocfnash, it seems your #10086 broke this; are you happy for me to move those lemmas to a new file?

@ocfnash
Copy link
Copy Markdown
Contributor

ocfnash commented Feb 1, 2024

@ocfnash, it seems your #10086 broke this; are you happy for me to move those lemmas to a new file?

Yes of course. Do you need any help? Which lemmas would you like to move?

@ghost ghost added blocked-by-other-PR This PR depends on another PR (this label is automatically managed by a bot) and removed blocked-by-other-PR This PR depends on another PR (this label is automatically managed by a bot) labels Feb 1, 2024
@ghost
Copy link
Copy Markdown

ghost commented Feb 1, 2024

@eric-wieser
Copy link
Copy Markdown
Member Author

bors merge

mathlib-bors bot pushed a commit that referenced this pull request Feb 1, 2024
As with #9950, this is motivated by:
* Getting `NNRat` closed to `norm_num`
* Being able to put an `nnrat_cast` field in `DivisionSemiring`s

This brings down the number of dependencies of `NNRat` by around 600.
@mathlib-bors
Copy link
Copy Markdown
Contributor

mathlib-bors bot commented Feb 1, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title refactor(Data/Rat/NNRat): move module and algebra instances [Merged by Bors] - refactor(Data/Rat/NNRat): move module and algebra instances Feb 1, 2024
@mathlib-bors mathlib-bors bot closed this Feb 1, 2024
@mathlib-bors mathlib-bors bot deleted the eric-wieser/move-nnrat-module branch February 1, 2024 22:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge This PR has been sent to bors.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants