Skip to content

[Merged by Bors] - chore: Replace (· op ·) a by (a op ·)#8843

Closed
YaelDillies wants to merge 16 commits intomasterfrom
cute_faces
Closed

[Merged by Bors] - chore: Replace (· op ·) a by (a op ·)#8843
YaelDillies wants to merge 16 commits intomasterfrom
cute_faces

Conversation

@YaelDillies
Copy link
Copy Markdown
Contributor

I used the regex \(\(· (.) ·\) (.)\), replacing with ($2 $1 ·).


Open in Gitpod

I used the regex `\(\(· (.) ·\) (.)\)`, replacing with `($2 $1 ·)`.
@YaelDillies YaelDillies added awaiting-review easy < 20s of review time. See the lifecycle page for guidelines. labels Dec 6, 2023
@eric-wieser
Copy link
Copy Markdown
Member

I don't think we want to do this; we're still holding out for an upstream fix that makes (· op ·) a elaborate as Op.op a, which is better than your new spelling. Another suggestion that was floated around was to add (*) as notation for unapplied HMul.hMul, etc.

cc @digama0, who had thoughts on a similar PR.

@YaelDillies
Copy link
Copy Markdown
Contributor Author

YaelDillies commented Dec 6, 2023

Surely if we can make (· op ·) elaborate as Op.op then we can make (a op ·) elaborate as Op.op a? We will eventually want this anyway as (a op ·) is more readable than (op) a (the left argument is on the right!).

And this is an improvement in the short term too, as can be evidenced by the number of dsimp I am removing.

@eric-wieser
Copy link
Copy Markdown
Member

Replacing (· * ·) with HMul.hMul is also an improvement (in terms of proof convenience) in the short term, but @digama0 did not like the idea of doing that. I guess really it depends if the plan is general eta reduction for · functions, or something special for binary operators.

@YaelDillies
Copy link
Copy Markdown
Contributor Author

I guess my point is that this PR is both a proof convenience and readability improvement in a way that other solutions will converge to if they improve both proof convenience and readability.

@digama0
Copy link
Copy Markdown
Member

digama0 commented Dec 7, 2023

Replacing (· * ·) with HMul.hMul is also an improvement (in terms of proof convenience) in the short term, but @digama0 did not like the idea of doing that. I guess really it depends if the plan is general eta reduction for · functions, or something special for binary operators.

The implementation leanprover/lean4#2267 does general eta reduction, there is nothing specific to binary operators in it.

I support this PR, it is a mathportism (I really wanted to do this directly in mathport but there were some complications IIRC). If I or anyone else manages to figure out how to fix leanprover/lean4#2267 then both variants will come out the same.

@hanwenzhu
Copy link
Copy Markdown
Collaborator

It seems like you can find a few more with \(\(· ([^ ]+) ·\) ([\w(].*?)\), though you have to filter the results manually

@YaelDillies YaelDillies added awaiting-CI This PR does not pass CI yet. This label is automatically removed once it does. and removed easy < 20s of review time. See the lifecycle page for guidelines. labels Dec 7, 2023
@github-actions github-actions bot removed the awaiting-CI This PR does not pass CI yet. This label is automatically removed once it does. label Dec 7, 2023
@ghost ghost added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Dec 7, 2023
Copy link
Copy Markdown
Member

@eric-wieser eric-wieser left a comment

Choose a reason for hiding this comment

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

bors d+

Thanks!

All my comments are optional, and are fine in a follow-up PR if you just want to get this merged before it goes stale.

@mathlib-bors
Copy link
Copy Markdown
Contributor

mathlib-bors bot commented Dec 7, 2023

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

@ghost ghost added delegated This pull request has been delegated to the PR author (or occasionally another non-maintainer). and removed awaiting-review labels Dec 7, 2023
@ghost ghost removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Dec 7, 2023
@YaelDillies
Copy link
Copy Markdown
Contributor Author

YaelDillies commented Dec 7, 2023

I read through the 4k hits of ·.*· manually to see whether I'd missed any (spoiler: I had) and I've now also made sure I didn't add unnecessary brackets within a type ascription. Will check everything builds, then send it to bors.

@YaelDillies
Copy link
Copy Markdown
Contributor Author

bors merge

@mathlib-bors
Copy link
Copy Markdown
Contributor

mathlib-bors bot commented Dec 8, 2023

Merge conflict.

@YaelDillies
Copy link
Copy Markdown
Contributor Author

bors merge

mathlib-bors bot pushed a commit that referenced this pull request Dec 8, 2023
I used the regex `\(\(· (.) ·\) (.)\)`, replacing with `($2 $1 ·)`.
@mathlib-bors
Copy link
Copy Markdown
Contributor

mathlib-bors bot commented Dec 8, 2023

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title chore: Replace (· op ·) a by (a op ·) [Merged by Bors] - chore: Replace (· op ·) a by (a op ·) Dec 8, 2023
@mathlib-bors mathlib-bors bot closed this Dec 8, 2023
@mathlib-bors mathlib-bors bot deleted the cute_faces branch December 8, 2023 11:33
awueth pushed a commit that referenced this pull request Dec 19, 2023
I used the regex `\(\(· (.) ·\) (.)\)`, replacing with `($2 $1 ·)`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

delegated This pull request has been delegated to the PR author (or occasionally another non-maintainer).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants