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: rename op_norm to opNorm #10185

Closed
wants to merge 6 commits into from
Closed

Conversation

sgouezel
Copy link
Contributor

@sgouezel sgouezel commented Feb 2, 2024


Open in Gitpod

@sgouezel sgouezel added awaiting-review The author would like community review of the PR t-analysis Analysis (normed *, calculus) labels Feb 2, 2024
@adomani
Copy link
Collaborator

adomani commented Feb 2, 2024

Picking up on this conversation, what would you think of adding after each modified #align x y something like:

theorem YYY ...

#align XXX YYY

@[deprecated YYY] alias YYYold := YYY -- deprecated on 2024/02/02

Catching changed aligns is relatively easy and then it is simply a matter of reading the git diff.

@sgouezel
Copy link
Contributor Author

sgouezel commented Feb 2, 2024

It would definitely be an improvement, but still not perfect because some lemmas are not backported from Lean 3 and therefore have no #align. If you know how to write a script reading git diff, it shouldn't be much more complicated to check for lines starting with theorem or lemma and see if the characters before the first ( or { or [ (i.e., the name) have changed, and then add the deprecation line at the first empty line after the theorem statement. Not completely fail-proof, but good enough for 99% of the cases I would say, and the remaining ones can be fixed by hand. Don't hesitate to experiment as much as you want on this branch! I'm marking it as WIP for now to let you experiment. You can put it back to awaiting review once you're happy with the result, or bored with the experiments!

@sgouezel sgouezel added WIP Work in progress and removed awaiting-review The author would like community review of the PR labels Feb 2, 2024
@adomani
Copy link
Collaborator

adomani commented Feb 2, 2024

Ok, I'm busy for about an hour, but will play with it a little afterwards!

@adomani
Copy link
Collaborator

adomani commented Feb 2, 2024

Btw, the main issue is not finding the start of the theorem, but the end.

@sgouezel
Copy link
Contributor Author

sgouezel commented Feb 2, 2024

Evyerthing between theorem or lemma and the first ( or { or [ should be the theorem name, right? And then go to the next empty line to put the deprecated statement? This breaks if there is an empty line in the proof, but this should be rare enough that it's not an issue, I guess.

@adomani
Copy link
Collaborator

adomani commented Feb 2, 2024

Yes, after theorem/lemma the first consecutive block of "mostly characters, digits, underscores and a few more symbols" is the theorem name. What does not necessarily work so well is finding the end of the whole declaration, not the end of the name, as not always declarations are separated by empty lines. However, this may be the case for the specific declarations touched in this PR (and for most declarations anyway), so I will use this heuristic, if needed.

@adomani
Copy link
Collaborator

adomani commented Feb 2, 2024

Ok, my previous comment was a little misguided: the deprecation statement does not need to be right after the statement to deprecate, although it is probably desirable that it is.

Anyway, I have a rough script for adding deprecations and the latest commit only adds the deprecations. Let's see if CI is happy and then what actual issues there are with the deprecations!

@adomani adomani added awaiting-review The author would like community review of the PR and removed WIP Work in progress labels Feb 2, 2024
@sgouezel
Copy link
Contributor Author

sgouezel commented Feb 3, 2024

This is awesome, thanks a lot!
Is the script easy to use? I can easily imagine that it would help me very often!

@adomani
Copy link
Collaborator

adomani commented Feb 3, 2024

The script is easy to use, but it assumes that every line that has a theorem/lemma in the diff corresponds to a name change to be deprecated. This applies if you change anything on that line, not necessarily the name! Also, it assumes that the new declarations appear in the same order as the old ones and in the same files.

I'll polish it up and make it available, though maybe tonight and after a little more testing.

@adomani
Copy link
Collaborator

adomani commented Feb 3, 2024

The script that I used is here.

To use it, copy it to your computer, make it executable and run it. You have the option of passing a commit hash that the script will use to figure out what has been deprecated. If you do not provide one, then the script will try to find the master commit from which you branched and use that. If you use a Unix-based system, the commands below should work (assuming that you saved the file in the root directory and called it add_deprecation.sh):

chmod u+x add_deprecation.sh
./add_deprecation.sh
##  you will be prompted to provide a commit hash, but the script suggests a guess
##  you can also pass the hash explicitly when you run the command

The heuristic that the script uses for finding which declarations to deprecate is very crude. This means that in order for the script to have a chance of producing a useful output, the PR should be carefully prepared to assist the script. Basically, your PR was perfect: just lemma renames and fixing up proofs. As soon as there are more changes happening on lines containing theorem/lemma, the script will get confused.

@urkud
Copy link
Member

urkud commented Feb 4, 2024

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 Feb 4, 2024
mathlib-bors bot pushed a commit that referenced this pull request Feb 4, 2024
Co-authored-by: adomani <adomani@gmail.com>
@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Feb 4, 2024

Build failed:

@sgouezel
Copy link
Contributor Author

sgouezel commented Feb 4, 2024

bors r+

mathlib-bors bot pushed a commit that referenced this pull request Feb 4, 2024
Co-authored-by: adomani <adomani@gmail.com>
@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Feb 4, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title chore: rename op_norm to opNorm [Merged by Bors] - chore: rename op_norm to opNorm Feb 4, 2024
@mathlib-bors mathlib-bors bot closed this Feb 4, 2024
@mathlib-bors mathlib-bors bot deleted the SG_op_norm branch February 4, 2024 13:39
Vierkantor pushed a commit that referenced this pull request Feb 5, 2024
Co-authored-by: adomani <adomani@gmail.com>
mathlib-bors bot pushed a commit that referenced this pull request Feb 8, 2024
Following [these](https://leanprover.zulipchat.com/#narrow/stream/287929-mathlib4/topic/Thank.20you.20for.20the.20deprecation.20warnings!/near/420078161) [Zulip](https://leanprover.zulipchat.com/#narrow/stream/270676-lean4/topic/machineApplicableDeprecated.20tag.20attribute/near/397630551) discussions, I realised that my deprecation script produced a deprecation syntax that did not allow for auto-replacement in Sébastien's #10185.

This PR fixes the deprecation statements, allowing self-correction: 119 times I replaced

`@[deprecated xxx] --> @[deprecated]`.
atarnoam pushed a commit that referenced this pull request Feb 9, 2024
Co-authored-by: adomani <adomani@gmail.com>
atarnoam pushed a commit that referenced this pull request Feb 9, 2024
Following [these](https://leanprover.zulipchat.com/#narrow/stream/287929-mathlib4/topic/Thank.20you.20for.20the.20deprecation.20warnings!/near/420078161) [Zulip](https://leanprover.zulipchat.com/#narrow/stream/270676-lean4/topic/machineApplicableDeprecated.20tag.20attribute/near/397630551) discussions, I realised that my deprecation script produced a deprecation syntax that did not allow for auto-replacement in Sébastien's #10185.

This PR fixes the deprecation statements, allowing self-correction: 119 times I replaced

`@[deprecated xxx] --> @[deprecated]`.
dagurtomas pushed a commit that referenced this pull request Mar 22, 2024
Following [these](https://leanprover.zulipchat.com/#narrow/stream/287929-mathlib4/topic/Thank.20you.20for.20the.20deprecation.20warnings!/near/420078161) [Zulip](https://leanprover.zulipchat.com/#narrow/stream/270676-lean4/topic/machineApplicableDeprecated.20tag.20attribute/near/397630551) discussions, I realised that my deprecation script produced a deprecation syntax that did not allow for auto-replacement in Sébastien's #10185.

This PR fixes the deprecation statements, allowing self-correction: 119 times I replaced

`@[deprecated xxx] --> @[deprecated]`.
mathlib-bors bot pushed a commit that referenced this pull request May 30, 2024
…3368)

Deprecations have been forgotten in #11486. We add them in this PR.

Methodology: start from the last commit of #11486, use the script by @adomani in #10185 (comment) to generate deprecation aliases (I am also adding an updated version of the script in a comment below) and commit. Then cherry pick this last commit on master and fix whatever has changed in between. This is not perfect as declarations which have been moved since #11486 won't get their deprecated alias, but this should be good enough.
callesonne pushed a commit that referenced this pull request Jun 4, 2024
…3368)

Deprecations have been forgotten in #11486. We add them in this PR.

Methodology: start from the last commit of #11486, use the script by @adomani in #10185 (comment) to generate deprecation aliases (I am also adding an updated version of the script in a comment below) and commit. Then cherry pick this last commit on master and fix whatever has changed in between. This is not perfect as declarations which have been moved since #11486 won't get their deprecated alias, but this should be good enough.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has been sent to bors. t-analysis Analysis (normed *, calculus)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants