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(data/polynomial): break up behemoth file #3407

Closed
wants to merge 44 commits into from

Conversation

jalex-stark
Copy link
Collaborator

@jalex-stark jalex-stark commented Jul 15, 2020

Polynomial refactor

The goal is to split data/polynomial.lean into several self-contained files in the same place. For the time being, the new place for all these files is data/polynomial/.
Future PRs may simplify proofs, remove duplicate lemmas, and move files out of the data directory.


Things to consider before merging

We split the contents of data/polynomial across 14 files. Some of them may want to be renamed and/or remixed, but unless there are any really obvious moves, I think we can defer that to future work.
All of them would be happy to have more detailed docstrings.

Does this PR make it easier or harder to find the lemmas? (as the author, I'm not sure)
Does this PR make the polynomial library easier to maintain? (my opinion is yes, definitely)

It will be mildly painful if this PR sits long enough for data/polynomial.lean to move ahead in master.

@jalex-stark jalex-stark added the WIP Work in progress label Jul 15, 2020
@jalex-stark jalex-stark changed the title chore(data/polynomial) chore(data/polynomial): break up behemoth file Jul 15, 2020
@jalex-stark jalex-stark added awaiting-review The author would like community review of the PR and removed WIP Work in progress labels Jul 15, 2020
Copy link
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 🎉 Big thanks!

bors merge

@github-actions github-actions bot added ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) and removed awaiting-review The author would like community review of the PR labels Jul 16, 2020
bors bot pushed a commit that referenced this pull request Jul 16, 2020
Polynomial refactor

The goal is to split `data/polynomial.lean` into several self-contained files in the same place. For the time being, the new place for all these files is `data/polynomial/`.
Future PRs may simplify proofs, remove duplicate lemmas, and move files out of the `data` directory.



Co-authored-by: Aaron Anderson <awainverse@gmail.com>
@bors
Copy link

bors bot commented Jul 16, 2020

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title chore(data/polynomial): break up behemoth file [Merged by Bors] - chore(data/polynomial): break up behemoth file Jul 16, 2020
@bors bors bot closed this Jul 16, 2020
@bors bors bot deleted the polynomial_refactor branch July 16, 2020 20:03
bors bot pushed a commit that referenced this pull request Jul 22, 2020
This builds on #3407, trying to get related material closer together. 

There shouldn't be any change to the set of declarations, just the order they come in and the imports required to get them.

The major changes are:
1. `data.polynomial.derivative` now has much weaker imports
2. generally, material has been moved "upwards" to the first place it can be done (a lot of material moved out of `data.polynomial.degree` into `data.polynomial.degree.basic` -- essentially `degree` is the material about `degree` that also needs `eval` and friends; a further rename might be appropriate)
3. some of the later material is no longer a big chain of linear dependencies, but compiles separately



Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
Co-authored-by: Chris Hughes <33847686+ChrisHughes24@users.noreply.github.com>
bors bot pushed a commit that referenced this pull request Jul 22, 2020
This builds on #3407, trying to get related material closer together. 

There shouldn't be any change to the set of declarations, just the order they come in and the imports required to get them.

The major changes are:
1. `data.polynomial.derivative` now has much weaker imports
2. generally, material has been moved "upwards" to the first place it can be done (a lot of material moved out of `data.polynomial.degree` into `data.polynomial.degree.basic` -- essentially `degree` is the material about `degree` that also needs `eval` and friends; a further rename might be appropriate)
3. some of the later material is no longer a big chain of linear dependencies, but compiles separately



Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
Co-authored-by: Chris Hughes <33847686+ChrisHughes24@users.noreply.github.com>
Co-authored-by: Chris Hughes <chrishughes24@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants