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

move(data/set/pointwise/interval): Sanitise imports #17347

Closed
wants to merge 14 commits into from

Conversation

YaelDillies
Copy link
Collaborator

Quite a lot of pure order theory files import data.set.intervals.unordered_interval, which itself imports data.set.intervals.image_preimage, which in turn imports a lot of algebra.
If we change the order of dependency between data.set.intervals.unordered_interval and data.set.intervals.image_preimage, we avoid algebra leaking into pure order theory and drastically reduce imports for many files.


@semorrison, I expect this makes #17340 useless.

Open in Gitpod

@YaelDillies YaelDillies added awaiting-CI The author would like to see what CI has to say before doing more work. awaiting-review The author would like community review of the PR t-algebra Algebra (groups, rings, fields etc) t-order Order hierarchy labels Nov 4, 2022
@semorrison
Copy link
Collaborator

Great, thanks, this looks promising. I'll check if there's any value to #17340 after this merges.

bors d+

@bors
Copy link

bors bot commented Nov 4, 2022

✌️ 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.

@github-actions github-actions bot added delegated The PR author may merge after reviewing final suggestions. and removed awaiting-review The author would like community review of the PR labels Nov 4, 2022
@semorrison semorrison requested a review from a team as a code owner November 4, 2022 12:33
@@ -3,6 +3,7 @@ Copyright (c) 2021 Yaël Dillies. All rights reserved.
Released under Apache 2.0 license as described in the file LICENSE.
Authors: Yaël Dillies
-/
import data.fin.basic
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just noting for later: this import is bogus. I remember adding the few fin lemmas at the bottom of the file because that was the first place where I had both access to covby and fin (due to the bogus import this PR is fixing), but those lemmas have nothing to do with succ/pred on nat.

@github-actions github-actions bot removed the awaiting-CI The author would like to see what CI has to say before doing more work. label Nov 5, 2022
@semorrison
Copy link
Collaborator

bors merge

@github-actions github-actions bot added the ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) label Nov 5, 2022
bors bot pushed a commit that referenced this pull request Nov 6, 2022
Quite a lot of pure order theory files import `data.set.intervals.unordered_interval`, which itself imports `data.set.intervals.image_preimage`, which in turn imports a lot of algebra.
If we change the order of dependency between `data.set.intervals.unordered_interval` and `data.set.intervals.image_preimage`, we avoid algebra leaking into pure order theory and drastically reduce imports for many files.



Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
@bors
Copy link

bors bot commented Nov 6, 2022

Build failed (retrying...):

@bors
Copy link

bors bot commented Nov 6, 2022

Canceled.

@semorrison
Copy link
Collaborator

There was a merge conflict with master, unfortunately.

bors merge p=25

bors bot pushed a commit that referenced this pull request Nov 6, 2022
Quite a lot of pure order theory files import `data.set.intervals.unordered_interval`, which itself imports `data.set.intervals.image_preimage`, which in turn imports a lot of algebra.
If we change the order of dependency between `data.set.intervals.unordered_interval` and `data.set.intervals.image_preimage`, we avoid algebra leaking into pure order theory and drastically reduce imports for many files.



Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
@bors
Copy link

bors bot commented Nov 6, 2022

Build failed:

@semorrison semorrison added awaiting-CI The author would like to see what CI has to say before doing more work. and removed ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) labels Nov 6, 2022
@semorrison
Copy link
Collaborator

This is going to merged as part of #17381. I'm closing now to conserve CI resources.

@semorrison semorrison closed this Nov 6, 2022
bors bot pushed a commit that referenced this pull request Nov 6, 2022
This is the supremum of

* #17376
* #17375
* #17374
* #17362
* #17355
* #17349
* #17347
* #17338
* #17336

with imports fixed up to accommodate all the overlapping changes to imports.





Co-authored-by: Yury G. Kudryashov <urkud@urkud.name>
Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
Co-authored-by: Yaël Dillies <yael.dillies@gmail.com>
bors bot pushed a commit that referenced this pull request Nov 6, 2022
This is the supremum of

* #17376
* #17375
* #17374
* #17362
* #17355
* #17349
* #17347
* #17338
* #17336

with imports fixed up to accommodate all the overlapping changes to imports.





Co-authored-by: Yury G. Kudryashov <urkud@urkud.name>
Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
Co-authored-by: Yaël Dillies <yael.dillies@gmail.com>
@YaelDillies YaelDillies deleted the pointwise_interval branch November 8, 2022 23:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-CI The author would like to see what CI has to say before doing more work. delegated The PR author may merge after reviewing final suggestions. t-algebra Algebra (groups, rings, fields etc) t-order Order hierarchy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants