[Merged by Bors] - chore: remove remaining stream-of-conciousness obtain syntax#13219
Closed
[Merged by Bors] - chore: remove remaining stream-of-conciousness obtain syntax#13219
Conversation
urkud
reviewed
May 26, 2024
Mathlib/Algebra/ContinuedFractions/Computation/Approximations.lean
Outdated
Show resolved
Hide resolved
Contributor
Author
|
I have addressed all comments. |
Member
|
Thanks! 🎉 |
mathlib-bors bot
pushed a commit
that referenced
this pull request
May 26, 2024
Concludes the work started in (TTBOMK) #11045 and #12850. To make sure this is exhaustive, I wrote a syntax linter for this, which I will propose separately. This uncovered some interesting interactions with the multi-goal linter: in a few instances, about using the newer syntax, the sub-goal focusing also became obsolete.
Contributor
Author
|
Thank you for the review! |
Contributor
|
Pull request successfully merged into master. Build succeeded: |
callesonne
pushed a commit
that referenced
this pull request
Jun 4, 2024
Concludes the work started in (TTBOMK) #11045 and #12850. To make sure this is exhaustive, I wrote a syntax linter for this, which I will propose separately. This uncovered some interesting interactions with the multi-goal linter: in a few instances, about using the newer syntax, the sub-goal focusing also became obsolete.
js2357
pushed a commit
that referenced
this pull request
Jun 18, 2024
Concludes the work started in (TTBOMK) #11045 and #12850. To make sure this is exhaustive, I wrote a syntax linter for this, which I will propose separately. This uncovered some interesting interactions with the multi-goal linter: in a few instances, about using the newer syntax, the sub-goal focusing also became obsolete.
mathlib-bors bot
pushed a commit
that referenced
this pull request
Jun 24, 2024
Just the syntax replacements are split into #13219. Most of its occurrences were removed in #11045 and #12850; #13219 removed the last instances. **Why is this removed?** In a nutshell, my understanding is that this is preferred against for similar reasons as #10534: - it's a Lean3-ism, which can be unlearned now - the syntax `obtain foo : type := proof` is slightly shorter; particularly so when the next tactic is `exact` - when using it as `obtain foo : type; · proof`, there is an intermediate state with multiple goals right before the focusing dot (This gets amplified with the in-flight "multiple goal linter", which in my perception seems generally desired - for many reasons, including teachability. Granted, the linter could be tweaked to not lint in this case... but by now, the "old" syntax is not clearly better.) - the old syntax *could* be slightly nicer when deferring goals: In the 30 replacements of the last PR, this occurred *twice* (i.e., very rarely). In both cases, `suffices` or `rsuffices` can be used, and would imho be clearer. Alternatively, the `obtain` tactic in Lean core could also be changed: this change does not block this at all, but moves forward with something that is doable within mathlib today.
kbuzzard
pushed a commit
that referenced
this pull request
Jun 26, 2024
Just the syntax replacements are split into #13219. Most of its occurrences were removed in #11045 and #12850; #13219 removed the last instances. **Why is this removed?** In a nutshell, my understanding is that this is preferred against for similar reasons as #10534: - it's a Lean3-ism, which can be unlearned now - the syntax `obtain foo : type := proof` is slightly shorter; particularly so when the next tactic is `exact` - when using it as `obtain foo : type; · proof`, there is an intermediate state with multiple goals right before the focusing dot (This gets amplified with the in-flight "multiple goal linter", which in my perception seems generally desired - for many reasons, including teachability. Granted, the linter could be tweaked to not lint in this case... but by now, the "old" syntax is not clearly better.) - the old syntax *could* be slightly nicer when deferring goals: In the 30 replacements of the last PR, this occurred *twice* (i.e., very rarely). In both cases, `suffices` or `rsuffices` can be used, and would imho be clearer. Alternatively, the `obtain` tactic in Lean core could also be changed: this change does not block this at all, but moves forward with something that is doable within mathlib today.
dagurtomas
pushed a commit
that referenced
this pull request
Jul 2, 2024
Just the syntax replacements are split into #13219. Most of its occurrences were removed in #11045 and #12850; #13219 removed the last instances. **Why is this removed?** In a nutshell, my understanding is that this is preferred against for similar reasons as #10534: - it's a Lean3-ism, which can be unlearned now - the syntax `obtain foo : type := proof` is slightly shorter; particularly so when the next tactic is `exact` - when using it as `obtain foo : type; · proof`, there is an intermediate state with multiple goals right before the focusing dot (This gets amplified with the in-flight "multiple goal linter", which in my perception seems generally desired - for many reasons, including teachability. Granted, the linter could be tweaked to not lint in this case... but by now, the "old" syntax is not clearly better.) - the old syntax *could* be slightly nicer when deferring goals: In the 30 replacements of the last PR, this occurred *twice* (i.e., very rarely). In both cases, `suffices` or `rsuffices` can be used, and would imho be clearer. Alternatively, the `obtain` tactic in Lean core could also be changed: this change does not block this at all, but moves forward with something that is doable within mathlib today.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Concludes the work started in (TTBOMK) #11045 and #12850.
To make sure this is exhaustive, I wrote a syntax linter for this, which I will propose separately.
This uncovered some interesting interactions with the multi-goal linter:
in a few instances, about using the newer syntax, the sub-goal focusing also became obsolete.