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.

Sign up for GitHub

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

Jump to bottom

[Merged by Bors] - chore(combinatorics/quiver/path): reduce imports #17216

Closed
wants to merge 3 commits into from
Closed

[Merged by Bors] - chore(combinatorics/quiver/path): reduce imports #17216

wants to merge 3 commits into from

Conversation

semorrison
Copy link
Collaborator

By using some lower-tech theorems, we can avoid importing the entire algebra.order hierarchy, significantly reducing the import dependencies of the basic category theory library.

Open in Gitpod

@semorrison semorrison added awaiting-review The author would like community review of the PR awaiting-CI The author would like to see what CI has to say before doing more work. labels Oct 27, 2022
@semorrison semorrison requested a review from a team as a code owner October 27, 2022 23:22
@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 Oct 28, 2022
@YaelDillies
Copy link
Collaborator

Looks fine, but is category_theory.path_category really part of "the basic category theory library"?

@semorrison
Copy link
Collaborator Author

Looks fine, but is category_theory.path_category really part of "the basic category theory library"?

I don't understand your comment. How does category_theory.path_category come into this? This means that a big chunk of the category theory library doesn't need to wait for the algebra.order hierarchy.

@YaelDillies
Copy link
Collaborator

It's the first file in category theory that imports combinatorics.quiver.path, right? So if it not a basic category theory file, then your claim is false.

@kmill
Copy link
Collaborator

kmill commented Oct 29, 2022

@YaelDillies The path category is for a pretty basic construction https://en.m.wikipedia.org/wiki/Free_category

@semorrison
Copy link
Collaborator Author

It's the first file in category theory that imports combinatorics.quiver.path, right? So if it not a basic category theory file, then your claim is false.

I'm so confused, and tbh a little annoyed at this conversation wasting time. It seems obvious that category_theory.path_category is part of the basic category theory library.

Please see https://leanprover.zulipchat.com/#narrow/stream/287929-mathlib4/topic/Porting.20category.20theory/near/306557065 for the discussion that prompted this PR. It includes a PDF of the import hierarchy after this PR. Before this PR it also included the entire algebra.order hierarchy.

bryangingechen
bryangingechen approved these changes Oct 30, 2022
View reviewed changes
Copy link
Collaborator

@bryangingechen bryangingechen 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 r+

@leanprover-community-bot-assistant leanprover-community-bot-assistant 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 Oct 30, 2022
bors bot pushed a commit that referenced this pull request Oct 30, 2022
@semorrison
chore(combinators/quiver/path): reduce imports (#17216)
7cbd861 
By using some lower-tech theorems, we can avoid importing the entire `algebra.order` hierarchy, significantly reducing the import dependencies of the basic category theory library.



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

bors bot commented Oct 30, 2022

Build failed:

@YaelDillies YaelDillies changed the title chore(combinators/quiver/path): reduce imports chore(combinatorics/quiver/path): reduce imports Oct 30, 2022
@YaelDillies
Copy link
Collaborator

This graph would have been helpful to include in the PR description in the first place 😄

@bryangingechen
Copy link
Collaborator

bors d+

@bors
Copy link

bors bot commented Oct 30, 2022

✌️ semorrison 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 the delegated The PR author may merge after reviewing final suggestions. label Oct 30, 2022
@bryangingechen bryangingechen added awaiting-author A reviewer has asked the author a question or requested changes delegated The PR author may merge after reviewing final suggestions. 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+`.) delegated The PR author may merge after reviewing final suggestions. labels Oct 30, 2022
@semorrison semorrison removed the awaiting-author A reviewer has asked the author a question or requested changes label Oct 31, 2022
@semorrison
Copy link
Collaborator Author

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 Oct 31, 2022
bors bot pushed a commit that referenced this pull request Oct 31, 2022
@semorrison
chore(combinatorics/quiver/path): reduce imports (#17216)
0cceefb 
By using some lower-tech theorems, we can avoid importing the entire `algebra.order` hierarchy, significantly reducing the import dependencies of the basic category theory library.



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

bors bot commented Oct 31, 2022

This PR was included in a batch that was canceled, it will be automatically retried

bors bot pushed a commit that referenced this pull request Oct 31, 2022
@semorrison
chore(combinatorics/quiver/path): reduce imports (#17216)
88bad67 
By using some lower-tech theorems, we can avoid importing the entire `algebra.order` hierarchy, significantly reducing the import dependencies of the basic category theory library.



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

bors bot commented Oct 31, 2022

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title chore(combinatorics/quiver/path): reduce imports [Merged by Bors] - chore(combinatorics/quiver/path): reduce imports Oct 31, 2022
@bors bors bot closed this Oct 31, 2022
@bors bors bot deleted the cat_th_imports branch October 31, 2022 15:55
@eric-wieser eric-wieser added the hacktoberfest-accepted Without this label hacktoberfest is scared off by bors label Nov 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bryangingechen bryangingechen bryangingechen approved these changes

Assignees
No one assigned
Labels
delegated The PR author may merge after reviewing final suggestions. hacktoberfest-accepted Without this label hacktoberfest is scared off by bors 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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@semorrison @YaelDillies @kmill @bryangingechen @eric-wieser @leanprover-community-bot-assistant