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] - feat: port Topology.PathConnected #2755

Closed
wants to merge 16 commits into from

Conversation

j-loreaux
Copy link
Collaborator

@j-loreaux j-loreaux commented Mar 9, 2023

The major thing to note is that I have replaced the CoeFun instance (which was p ↦ p.toContinuousMap.toFun) with FunLike.coe, but previously Path did not have a FunLike instance, so I have added a ContinuousMapClass instance. These are defeq.

This also meant there would have been two coercions from Path x y to C(I, X), namely, the one which was originally declared in this file (:= p ↦ p.toContinuousMap) and the one inherited from ContinuousMapClass. As a result I have opted for the deleting the former in favor of the latter. These are not definitionally equal, but with the rewrite lemmas in place, there seem to be no real issues.

Note: the biggest reason for switching to FunLike was that the previous coercions were causing absolute hell early in the file with timeouts. If you want to see what I mean, checkout an early commit and attempt to start fixing.


Open in Gitpod

@j-loreaux j-loreaux added WIP Work in progress mathlib-port This is a port of a theory file from mathlib. help-wanted The author needs attention to resolve issues labels Mar 9, 2023
@j-loreaux j-loreaux removed the help-wanted The author needs attention to resolve issues label Mar 21, 2023
@j-loreaux j-loreaux added awaiting-review The author would like community review of the PR and removed WIP Work in progress labels Mar 21, 2023
@ChrisHughes24
Copy link
Member

bors r+

@github-actions github-actions bot added ready-to-merge This PR has been sent to bors. and removed awaiting-review The author would like community review of the PR labels Mar 23, 2023
bors bot pushed a commit that referenced this pull request Mar 23, 2023
The major thing to note is that I have replaced the `CoeFun` instance (which was `p ↦ p.toContinuousMap.toFun`) with `FunLike.coe`, but previously `Path` did not have a `FunLike` instance, so I have added a `ContinuousMapClass` instance. These are defeq.

This also meant there would have been *two* coercions from `Path x y` to `C(I, X)`, namely, the one which was originally declared in this file (`:= p ↦ p.toContinuousMap`) and the one inherited from `ContinuousMapClass`. As a result I have opted for the deleting the former in favor of the latter. These are *not* definitionally equal, but with the rewrite lemmas in place, there seem to be no real issues.

Note: the biggest reason for switching to `FunLike` was that the previous coercions were causing absolute hell early in the file with timeouts. If you want to see what I mean, checkout an early commit and attempt to start fixing. 



Co-authored-by: Moritz Firsching <firsching@google.com>
@bors
Copy link

bors bot commented Mar 23, 2023

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat: port Topology.PathConnected [Merged by Bors] - feat: port Topology.PathConnected Mar 23, 2023
@bors bors bot closed this Mar 23, 2023
@bors bors bot deleted the port/Topology.PathConnected branch March 23, 2023 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mathlib-port This is a port of a theory file from mathlib. ready-to-merge This PR has been sent to bors.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants