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: resolve (coe : ℤ → α) to ((↑) : ℤ → α) porting notes #11250

Closed
wants to merge 3 commits into from

Conversation

pitmonticone
Copy link
Collaborator

@pitmonticone pitmonticone commented Mar 8, 2024

Resolves porting notes claiming "change (coe : ℤ → α) to ((↑) : ℤ → α)" by substituting Int.cast with (↑).

@pitmonticone pitmonticone added awaiting-review easy < 20s of review time. See the lifecycle page for guidelines. porting-notes Mathlib3 to Mathlib4 porting notes. labels Mar 8, 2024
@eric-wieser
Copy link
Member

Let's just fix these; all the Int.casts should be (↑) which is the spelling of coe in Lean 4.

@pitmonticone
Copy link
Collaborator Author

pitmonticone commented Mar 8, 2024

@eric-wieser Fixed and edited the issue and PR titles.

@pitmonticone pitmonticone changed the title chore: classify (coe : ℤ → α) to (Int.cast : ℤ → α) porting notes chore: classify (coe : ℤ → α) to ((↑) : ℤ → α) porting notes Mar 8, 2024
Comment on lines -681 to 682
-- Porting note: changed `(coe : ℤ → α)` to `(Int.cast : ℤ → α)`
theorem gc_coe_floor : GaloisConnection (Int.cast : ℤ → α) floor :=
theorem gc_coe_floor : GaloisConnection ((↑) : ℤ → α) floor :=
FloorRing.gc_coe_floor
#align int.gc_coe_floor Int.gc_coe_floor
Copy link
Member

@eric-wieser eric-wieser Mar 8, 2024

Choose a reason for hiding this comment

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

@YaelDillies, do you know why we have both Int.gc_coe_floor and FloorRing.gc_coe_floor? Was one about a different coercion in mathlib3?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it was just for discoverability as the operator is Int.floor, not FloorRing.floor.

@pitmonticone pitmonticone changed the title chore: classify (coe : ℤ → α) to ((↑) : ℤ → α) porting notes chore: resolve (coe : ℤ → α) to ((↑) : ℤ → α) porting notes Mar 8, 2024
@eric-wieser
Copy link
Member

bors d=@YaelDillies

@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Mar 8, 2024

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

@YaelDillies
Copy link
Collaborator

Why not just remove the porting notes?

@pitmonticone
Copy link
Collaborator Author

Hi @YaelDillies, I’ve removed them and updated title and description.

@YaelDillies
Copy link
Collaborator

No, that's not what I meant. I meant "Why are you replacing (\u) with Int.cast? Can't we just delete the porting notes?". (\u) is exactly the spelling corresponding to the Lean 3 coe, and I see specifying what coercion we mean as a regression (after all, the point of the coercion is to be found automatically). Very rarely do you need to specify what coercion you're talking about (I just hit such a case on branch nnrat_cast), but these are manifestly not such cases.

@eric-wieser
Copy link
Member

@YaelDillies, I don't understand your comment, it seems to be talking about a patch that doesn't exist in this PR.

Copy link
Collaborator

@YaelDillies YaelDillies left a comment

Choose a reason for hiding this comment

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

Oh I see I swapped the diff colors in my head 🙈

bors merge

mathlib-bors bot pushed a commit that referenced this pull request Mar 9, 2024
Resolves porting notes claiming "change `(coe : ℤ → α)` to `((↑) : ℤ → α)`" by substituting `Int.cast` with `(↑)`.
@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Mar 9, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title chore: resolve (coe : ℤ → α) to ((↑) : ℤ → α) porting notes [Merged by Bors] - chore: resolve (coe : ℤ → α) to ((↑) : ℤ → α) porting notes Mar 9, 2024
@mathlib-bors mathlib-bors bot closed this Mar 9, 2024
@mathlib-bors mathlib-bors bot deleted the pitmonticone/change-coe-cast branch March 9, 2024 11:33
@pitmonticone
Copy link
Collaborator Author

Closes #11249

kbuzzard pushed a commit that referenced this pull request Mar 12, 2024
Resolves porting notes claiming "change `(coe : ℤ → α)` to `((↑) : ℤ → α)`" by substituting `Int.cast` with `(↑)`.
dagurtomas pushed a commit that referenced this pull request Mar 22, 2024
Resolves porting notes claiming "change `(coe : ℤ → α)` to `((↑) : ℤ → α)`" by substituting `Int.cast` with `(↑)`.
utensil pushed a commit that referenced this pull request Mar 26, 2024
Resolves porting notes claiming "change `(coe : ℤ → α)` to `((↑) : ℤ → α)`" by substituting `Int.cast` with `(↑)`.
Louddy pushed a commit that referenced this pull request Apr 15, 2024
Resolves porting notes claiming "change `(coe : ℤ → α)` to `((↑) : ℤ → α)`" by substituting `Int.cast` with `(↑)`.
uniwuni pushed a commit that referenced this pull request Apr 19, 2024
Resolves porting notes claiming "change `(coe : ℤ → α)` to `((↑) : ℤ → α)`" by substituting `Int.cast` with `(↑)`.
callesonne pushed a commit that referenced this pull request Apr 22, 2024
Resolves porting notes claiming "change `(coe : ℤ → α)` to `((↑) : ℤ → α)`" by substituting `Int.cast` with `(↑)`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
delegated easy < 20s of review time. See the lifecycle page for guidelines. porting-notes Mathlib3 to Mathlib4 porting notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants