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(data/fintype): move results depending on big_operators #2044

Merged
merged 3 commits into from
Feb 25, 2020

Conversation

semorrison
Copy link
Collaborator

This is just rearranging some results. In particular, this PR makes data.fintype not depend on algebra.big_operators, and thus needs to move a few results to a new file data.fintype.card.

I've been wanting to do this for a long time, as I perpetually notice that "easy" bits of the library seem to require recompiling the whole algebraic hierarchy when something in basic tactics changes, and I'm pretty sure the dependency is often via fintype importing algebra.big_operators, which then imports everything else.

I appreciate that reducing compile times is not the greatest principle for deciding how files get divided up ... but compile times are still a huge hassle at the moment.

@jcommelin
Copy link
Member

Looks good to me.

@urkud
Copy link
Member

urkud commented Feb 24, 2020

Another source of recompiles: data.list.basic depends on data.fin depends on data.nat.basic depends on algebra.ordered_ring.

@semorrison
Copy link
Collaborator Author

Oof, yes, let's break the chain of imports there. I don't think that "basic facts about lists" should require you to know about ordered rings. Not in this PR, however.

@digama0
Copy link
Member

digama0 commented Feb 24, 2020

That's not actually an unreasonable import. algebra.ordered_ring is the most basic file about ordered rings, just the basic lemmas about le and add and stuff, no theory. (If it becomes more than that, that's where we should focus attention.)

@semorrison
Copy link
Collaborator Author

@jcommelin, want to mark as ready to merge?

@semorrison semorrison added the awaiting-review The author would like community review of the PR label Feb 25, 2020
@jcommelin jcommelin 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 Feb 25, 2020
@mergify mergify bot merged commit f77cb57 into master Feb 25, 2020
@mergify mergify bot deleted the fintype_split branch February 25, 2020 10:40
anrddh pushed a commit to anrddh/mathlib that referenced this pull request May 15, 2020
…ver-community#2044)

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
anrddh pushed a commit to anrddh/mathlib that referenced this pull request May 16, 2020
…ver-community#2044)

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.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

4 participants