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] - chore: Forward-port leanprover-community/mathlib#19230 #5907

Closed
wants to merge 2 commits into from

Conversation

YaelDillies
Copy link
Collaborator

@YaelDillies YaelDillies commented Jul 14, 2023

This doesn't forward-port the removal of .{u} as this doesn't actually change the type, and just results in .{u_1} being implied instead.


Open in Gitpod

This forward-ports the revert (leanprover-community/mathlib#19230) of changes that were never forward-ported (leanprover-community/mathlib#19153). Therefore only the SHA needs updating.

@YaelDillies YaelDillies added awaiting-review The author would like community review of the PR easy < 20s of review time. See the lifecycle page for guidelines. mathlib3-pair This PR is a forward-port of a mathlib3 PR or part of one, either under review or recently merged labels Jul 14, 2023
@urkud
Copy link
Member

urkud commented Jul 14, 2023

Thanks! 🎉
bors merge

@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 Jul 14, 2023
@eric-wieser
Copy link
Member

bors r-

@YaelDillies, I'm confused by this claim. If you look at the file in its current state, it mentions (X : Scheme.{u}) not (X : Scheme). Are you sure we shouldn't be forward-porting the removal of the {u}?

@bors
Copy link

bors bot commented Jul 14, 2023

Canceled.

@YaelDillies
Copy link
Collaborator Author

@semorrison, can you explain more? The history for the mathlib4 file contains no PR that claims to forward-port leanprover-community/mathlib#19153, so I am confusion.

@eric-wieser
Copy link
Member

I suspect it was forward-ported while the file was still in progress

@eric-wieser
Copy link
Member

We should either:

  • Update the mathlib4 file to reflect the content that mathport is outputting, whether or not the diff makes sense
  • Update the mathlib3 file to reflect the version that was put into mathlib4 (i.e. revert the revert)

Probably only Scott can say which of these is correct

@eric-wieser eric-wieser added awaiting-review The author would like community review of the PR and removed ready-to-merge This PR has been sent to bors. labels Jul 16, 2023
@semorrison
Copy link
Contributor

These file have already diverged from mathlib3 due to the UnivLE experiments. Any discrepancies are mathlib3's problem, at this point. :-)

@semorrison
Copy link
Contributor

We tried to forward port 19153, failed, and so reverted it back in mathlib3. Instead we've introduced the UnivLE mechanism to deal with universe inequalities where Lean 4 can't cope with the max u v unification problems.

@semorrison semorrison added awaiting-author A reviewer has asked the author a question or requested changes and removed awaiting-review The author would like community review of the PR labels Jul 16, 2023
@YaelDillies
Copy link
Collaborator Author

So should this file appear on out-of-sync, yes or no? Scott, as the author of the mathlib changes, this is your responsability.

@semorrison
Copy link
Contributor

I'll make a SHA only PR now.

@semorrison semorrison closed this Jul 16, 2023
@semorrison semorrison reopened this Jul 16, 2023
@semorrison
Copy link
Contributor

This is already a SHA only PR... I don't understand why the bors merge was cancelled in the first place.

bors merge

@semorrison semorrison removed the awaiting-author A reviewer has asked the author a question or requested changes label Jul 16, 2023
@github-actions github-actions bot added the ready-to-merge This PR has been sent to bors. label Jul 16, 2023
bors bot pushed a commit that referenced this pull request Jul 16, 2023
This forward-ports the revert (leanprover-community/mathlib#19230) of changes that were never forward-ported (leanprover-community/mathlib#19153). Therefore only the SHA needs updating.
@bors
Copy link

bors bot commented Jul 16, 2023

Canceled.

@eric-wieser
Copy link
Member

bors merge

I don't know why we didn't just forward-port the diff here...

bors bot pushed a commit that referenced this pull request Jul 16, 2023
This doesn't forward-port the removal of `.{u}` as this doesn't actually change the type, and just results in `.{u_1}` being implied instead.



Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
@bors
Copy link

bors bot commented Jul 16, 2023

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

@eric-wieser eric-wieser changed the title chore: Forward-port #19230 chore: Forward-port leanprover-community/mathlib#19230 Jul 16, 2023
bors bot pushed a commit that referenced this pull request Jul 16, 2023
This doesn't forward-port the removal of `.{u}` as this doesn't actually change the type, and just results in `.{u_1}` being implied instead.



Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
@bors
Copy link

bors bot commented Jul 16, 2023

Pull request successfully merged into master.

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot changed the title chore: Forward-port leanprover-community/mathlib#19230 [Merged by Bors] - chore: Forward-port leanprover-community/mathlib#19230 Jul 16, 2023
@bors bors bot closed this Jul 16, 2023
@bors bors bot deleted the revert_19153 branch July 16, 2023 13:05
semorrison pushed a commit that referenced this pull request Aug 14, 2023
This doesn't forward-port the removal of `.{u}` as this doesn't actually change the type, and just results in `.{u_1}` being implied instead.



Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
easy < 20s of review time. See the lifecycle page for guidelines. mathlib3-pair This PR is a forward-port of a mathlib3 PR or part of one, either under review or recently merged 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