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

Refactor precomposition #937

Merged
merged 20 commits into from
Nov 24, 2023
Merged

Refactor precomposition #937

merged 20 commits into from
Nov 24, 2023

Conversation

EgbertRijke
Copy link
Collaborator

This PR refactors precomposition, following difficulties that people have been facing around where to put stuff involving precomposition, and cyclic dependencies that are too easily arising. In fact, it was quite a knot to untie. At the core of the knot was what is now called the (dependent) universal property of equivalences, which is in its own separate file now. I created tables, and did some minor code formatting, but please note that this is not the main purpose of this PR.

@fredrik-bakke
Copy link
Collaborator

This is certainly a nice improvement to the library, but I worry that the combination of refactoring precomposition and removing unused imports is going to lead to a lot of merge conflicts for other PRs. Therefore I wonder if it is best to hold off on merging this PR until we have gotten more of the other PRs in the pipeline merged.

@EgbertRijke EgbertRijke enabled auto-merge (squash) November 24, 2023 20:31
@EgbertRijke
Copy link
Collaborator Author

This is certainly a nice improvement to the library, but I worry that the combination of refactoring precomposition and removing unused imports is going to lead to a lot of merge conflicts for other PRs. Therefore I wonder if it is best to hold off on merging this PR until we have gotten more of the other PRs in the pipeline merged.

I merged all the PRs that were marked as ready that were opened prior to this one, and a few more. So now I think we shouldn't hold off on merging this one anymore.

@EgbertRijke EgbertRijke disabled auto-merge November 24, 2023 21:19
Copy link
Collaborator

@VojtechStep VojtechStep left a comment

Choose a reason for hiding this comment

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

Thanks for taking this on!

@EgbertRijke
Copy link
Collaborator Author

If you think it’s done please use that merge button:) I need to train you guys to use it more 😄

@VojtechStep VojtechStep merged commit 18fd380 into UniMath:master Nov 24, 2023
4 checks passed
@EgbertRijke
Copy link
Collaborator Author

And thank you for the review btw! You definitely caught some things that needed fixing!

@VojtechStep
Copy link
Collaborator

My pleasure!

fredrik-bakke pushed a commit to fredrik-bakke/agda-unimath that referenced this pull request Nov 26, 2023
This PR refactors precomposition, following difficulties that people
have been facing around where to put stuff involving precomposition, and
cyclic dependencies that are too easily arising. In fact, it was quite a
knot to untie. At the core of the knot was what is now called the
(dependent) universal property of equivalences, which is in its own
separate file now. I created tables, and did some minor code formatting,
but please note that this is not the main purpose of this PR.
fredrik-bakke pushed a commit to fredrik-bakke/agda-unimath that referenced this pull request Nov 29, 2023
This PR refactors precomposition, following difficulties that people
have been facing around where to put stuff involving precomposition, and
cyclic dependencies that are too easily arising. In fact, it was quite a
knot to untie. At the core of the knot was what is now called the
(dependent) universal property of equivalences, which is in its own
separate file now. I created tables, and did some minor code formatting,
but please note that this is not the main purpose of this PR.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants