[Merged by Bors] - feat: warn on deprecated declarations in scripts/check-yaml#38313
[Merged by Bors] - feat: warn on deprecated declarations in scripts/check-yaml#38313thorimur wants to merge 11 commits intoleanprover-community:masterfrom
scripts/check-yaml#38313Conversation
PR summary 188cb08f82Import changes for modified filesNo significant changes to the import graph Import changes for all files
Declarations diffNo declarations were harmed in the making of this PR! 🐙 You can run this locally as follows## summary with just the declaration names:
./scripts/pr_summary/declarations_diff.sh <optional_commit>
## more verbose report:
./scripts/pr_summary/declarations_diff.sh long <optional_commit>The doc-module for No changes to technical debt.This script lives in the
|
Updates deprecated declarations in `1000.yaml`, `overview.yaml`, and `undergrad.yaml`. Found with #38313. Note: in 1000.yaml, the removed declaration `ProbabilityTheory.binomial_tendsto_poissonPMFReal_atTop` has been deprecated in favor of the adjacent already-present declaration `ProbabilityTheory.tendsto_choose_mul_pow_of_tendsto_mul_atTop`. Co-authored-by: thorimur <thomasmurrills@gmail.com>
|
This PR/issue depends on:
|
grunweg
left a comment
There was a problem hiding this comment.
Thanks, this is great to have and much cleaner! I'm not an expert on the initSearchPath change (and would appreciate a second pair of eyes on it); the rest is straightforward and much nicer.
maintainer merge?
|
🚀 Pull request has been placed on the maintainer queue by grunweg. |
adomani
left a comment
There was a problem hiding this comment.
Thanks, this looks great!
Would it be possible to add a test for this code? Maybe by extending the databases list to contain a test that exercises the various deprecated/missing possibilities?
Scripts that are not in Mathlib have a tendency of breaking and go unnoticed...
|
Whether or not adding a test is feasible, I'll leave the choice to you! bors d+ |
|
✌️ thorimur can now approve this pull request. To approve and merge a pull request, simply reply with |
I'd love to add a test, but it is more than a little awkward...we'd have to adjust the python script to output a new json file, I think (or permanently include a json somewhere?). It's taking me a little bit to figure out what the right way to test this is, so maybe let's merge this so that no more deprecations creep in in the meantime, then PR the test separately if we can figure it out! |
Co-authored-by: damiano <adomani@gmail.com>
|
bors r+ |
This PR updates `scripts/check-yaml.lean` to warn on deprecated declarations. It also cleans up some code: `initSearchPath` does exactly what the removed code plus `CoreM.withImportModules` does (and as far as I can see we no longer need `CoreM`). Also modulizes and removes the dependence on Mathlib. Co-authored-by: thorimur <thomasmurrills@gmail.com>
|
Pull request successfully merged into master. Build succeeded: |
scripts/check-yamlscripts/check-yaml
|
Here's an idea about testing: you could
I even tend towards "use an actual deprecated item", and adding a detailed comment in the test what it's testing. Every six months, the test will have to be updated to use a new deprecated item. This is not great, but I'd say it's good enough. Actually, thinking about it more: you could have a test file defining a test-only deprecated item, and use that for the test... |
This PR updates
scripts/check-yaml.leanto warn on deprecated declarations.It also cleans up some code:
initSearchPathdoes exactly what the removed code plusCoreM.withImportModulesdoes (and as far as I can see we no longer needCoreM). Also modulizes and removes the dependence on Mathlib.*.yamls #38312