Skip to content

[Merged by Bors] - chore: cleanup set_option linter.deprecated#19701

Closed
kim-em wants to merge 5 commits intomasterfrom
linter_deprecated
Closed

[Merged by Bors] - chore: cleanup set_option linter.deprecated#19701
kim-em wants to merge 5 commits intomasterfrom
linter_deprecated

Conversation

@kim-em
Copy link
Copy Markdown
Contributor

@kim-em kim-em commented Dec 3, 2024

Remove several uses of deprecated lemmas in non-deprecated lemmas.

Also convert all set_option linter.deprecated false to set_option linter.deprecated false in, so it is easier for us to count via regex what remains.

I think

set_option linter\.deprecated false.*(?:(?:\n/--.*?-/\n(?!.*deprecated))|\n(?!.*deprecated|/--))

is the correct regex to use now: it accounts properly for intervening doc-strings. It now reports that there are only three one! files that use set_option linter.deprecated false on non-deprecated declarations, namely:

  • Mathlib.Data.List.Permutation (fixed by moving some List theorems earlier)
  • Mathlib.LinearAlgebra.CliffordAlgebra.Grading (replace AlgHom.map_zero with just map_zero; only costs 2000 heartbeats)
  • Mathlib.SetTheory.Ordinal.Arithmetic (which is a mess)

@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 3, 2024

PR summary 9f175f1354

Import changes for modified files

Dependency changes

File Base Count Head Count Change
Mathlib.Algebra.BigOperators.Group.List 386 388 +2 (+0.52%)
Mathlib.SetTheory.Cardinal.Basic 644 646 +2 (+0.31%)
Mathlib.SetTheory.Ordinal.Arithmetic 653 655 +2 (+0.31%)
Mathlib.SetTheory.Ordinal.Rank 654 656 +2 (+0.31%)
Mathlib.Order.Extension.Well 656 658 +2 (+0.30%)
Mathlib.SetTheory.Ordinal.FixedPoint 657 659 +2 (+0.30%)
Mathlib.SetTheory.Cardinal.Aleph 677 679 +2 (+0.30%)
Mathlib.LinearAlgebra.Prod 743 745 +2 (+0.27%)
Mathlib.Topology.Order.Basic 771 773 +2 (+0.26%)
Mathlib.LinearAlgebra.CliffordAlgebra.Grading 1405 1406 +1 (+0.07%)
Mathlib.Analysis.Analytic.Composition 1481 1482 +1 (+0.07%)
Import changes for all files
Files Import difference
There are 3768 files with changed transitive imports taking up over 163177 characters: this is too many to display!
You can run scripts/import_trans_difference.sh all locally to see the whole output.

Declarations diff

No declarations were harmed in the making of this PR! 🐙

You can run this locally as follows
## summary with just the declaration names:
./scripts/declarations_diff.sh <optional_commit>

## more verbose report:
./scripts/declarations_diff.sh long <optional_commit>

The doc-module for script/declarations_diff.sh contains some details about this script.


Increase in tech debt: (relative, absolute) = (32.00, 0.37)
Current number Change Type
87 32 disabled deprecation lints

Current commit 9f175f1354
Reference commit 72b0941b7c

You can run this locally as

./scripts/technical-debt-metrics.sh pr_summary
  • The relative value is the weighted sum of the differences with weight given by the inverse of the current value of the statistic.
  • The absolute value is the relative value divided by the total sum of the inverses of the current values (i.e. the weighted average of the differences).

@kim-em
Copy link
Copy Markdown
Contributor Author

kim-em commented Dec 3, 2024

There are some slight import increases because I moved some List lemmas around, but these will be undone by leanprover/lean4#6294.

Copy link
Copy Markdown
Contributor

@grunweg grunweg left a comment

Choose a reason for hiding this comment

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

Thanks!
maintainer merge

(I trust you that all set_option linter.deprecated in are actually required. I only checked some of them.)

@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 3, 2024

🚀 Pull request has been placed on the maintainer queue by grunweg.

@github-actions github-actions bot added the maintainer-merge A reviewer has approved the changed; awaiting maintainer approval. label Dec 3, 2024
Copy link
Copy Markdown
Member

@jcommelin jcommelin left a comment

Choose a reason for hiding this comment

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

Thanks 🎉

bors merge

@ghost ghost added the ready-to-merge This PR has been sent to bors. label Dec 3, 2024
mathlib-bors bot pushed a commit that referenced this pull request Dec 3, 2024
Remove several uses of deprecated lemmas in non-deprecated lemmas.

Also convert all `set_option linter.deprecated false` to `set_option linter.deprecated false in`, so it is easier for us to count via regex what remains.

I think
```
set_option linter\.deprecated false.*(?:(?:\n/--.*?-/\n(?!.*deprecated))|\n(?!.*deprecated|/--))
```
is the correct regex to use now: it accounts properly for intervening doc-strings. It now reports that there are only ~~three~~ one! file~~s~~ that use `set_option linter.deprecated false` on non-deprecated declarations, namely:

* ~~Mathlib.Data.List.Permutation~~ (fixed by moving some List theorems earlier)
* ~~Mathlib.LinearAlgebra.CliffordAlgebra.Grading~~ (replace `AlgHom.map_zero` with just `map_zero`; only costs 2000 heartbeats)
* Mathlib.SetTheory.Ordinal.Arithmetic (which is a mess)
@mathlib-bors
Copy link
Copy Markdown
Contributor

mathlib-bors bot commented Dec 3, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title chore: cleanup set_option linter.deprecated [Merged by Bors] - chore: cleanup set_option linter.deprecated Dec 3, 2024
@mathlib-bors mathlib-bors bot closed this Dec 3, 2024
@mathlib-bors mathlib-bors bot deleted the linter_deprecated branch December 3, 2024 11:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintainer-merge A reviewer has approved the changed; awaiting maintainer approval. ready-to-merge This PR has been sent to bors.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants