-
Notifications
You must be signed in to change notification settings - Fork 975
[Merged by Bors] - chore: remove backward.privateInPublic from lakefile #33034
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
Conversation
Adds set_option statements to allow private declarations to be accessed publicly in 20+ files, primarily in Algebra/Category and related modules. This is part of the migration away from the global backward.privateInPublic option. Each private declaration that is accessed publicly gets: - set_option backward.privateInPublic true in (before the private decl) - Both set_options (including warn false) before public callers Files modified include category theory structures (AlgCat, ModuleCat, etc.), algebraic structures (DirectSum, FreeAlgebra), and basic definitions. Part of the work to address privateInPublic warnings across mathlib.
Adds set_option statements for private helper functions in: - Algebra/Group/Defs.lean (inv_eq_of_mul) - Algebra/Group/End.lean (inv_aux, pow_aux, zpow_aux) - Algebra/Group/Subsemigroup/Operations.lean (srange_mk_aux_mul) Part of migrating away from global backward.privateInPublic option.
Add set_option statements for private inv_eq_of_mul helper function.
Fixes backward.privateInPublic errors in: - Mathlib/Order/Antisymmetrization.lean (liftFun_antisymmRel) - Mathlib/Data/List/SplitBy.lean (nil_notMem_splitByLoop alias) - Mathlib/Data/Nat/Prime/Defs.lean (minFac_has_prop) - Mathlib/Order/BooleanAlgebra/Basic.lean (sdiff_sup_self') - Mathlib/GroupTheory/OreLocalization/Basic.lean (smul'') - Mathlib/RingTheory/OreLocalization/Basic.lean (add, nsmul) - Mathlib/Logic/Encodable/Basic.lean (good, decidable_good) - Mathlib/Order/Category/Preord.lean (Hom.mk constructor) - Mathlib/CategoryTheory/IsConnected.lean (liftToDiscrete, factorThroughDiscrete) - Mathlib/CategoryTheory/Monoidal/Rigid/Braided.lean (coevaluation_evaluation_braided', evaluation_coevaluation_braided') - Mathlib/Order/Lattice/Congruence.lean (transitive) - Mathlib/Data/Finset/Union.lean (pairwiseDisjoint_fibers) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Fixes backward.privateInPublic errors in: - Mathlib/Order/Category/PartOrd.lean (Hom.mk constructor) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…es (batch 1) Fixes backward.privateInPublic errors in: - Mathlib/Order/Category/Lat.lean (Hom.mk constructor) - Mathlib/Order/Category/BddOrd.lean (Hom.mk constructor) - Mathlib/Order/Category/LinOrd.lean (Hom.mk constructor) - Mathlib/Order/Category/DistLat.lean (Hom.mk constructor) - Mathlib/Order/Category/BddLat.lean (Hom.mk constructor) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…es (batch 2) Fixes backward.privateInPublic errors in: - Mathlib/Order/Category/BoolAlg.lean (Hom.mk constructor) - Mathlib/Order/Category/FinBddDistLat.lean (Hom.mk constructor) - Mathlib/Order/Category/PartOrdEmb.lean (Hom.mk constructor) - Mathlib/Order/Category/HeytAlg.lean (Hom.mk constructor) - Mathlib/Order/Category/Frm.lean (Hom.mk constructor) - Mathlib/Order/Category/BddDistLat.lean (Hom.mk constructor) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Move set_option before doc comment and @[ext] attribute to fix "unexpected token 'set_option'" error. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add set_option to allow public declaration typeToCatObjectsAdj to access private definitions typeToCatObjectsAdjHomEquiv and typeToCatObjectsAdjCounitApp. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Move set_option statements before @[to_additive] attributes to fix "unexpected token 'set_option'" errors. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add set_option to allow usage of private Hom.mk constructor in categoryStruct and mkHom. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Move set_option statements before @[to_additive] attributes to fix "unexpected token 'set_option'" errors. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add set_option to allow access to private zero_def and one_def in Cauchy.ring and Cauchy.commRing instances. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add set_option to allow access to private giAux in CompleteLattice and Frame.MinimalAxioms instances. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add set_option for CommMonCat.Hom private constructor usage. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add set_option for private lift₂Aux and globalEquivAux definitions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add set_option for private widePullbackShapeEquivObj and widePullbackShapeEquivMap definitions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add set_option for CommGrpCat.Hom private constructor usage. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add set_option for private natEmbeddingAux_injective theorem. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add set_option for private supOfSeq, coprimes, and pairwise_coprime_coprimes definitions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add set_option for private fromVector and pairOther definitions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add set_option for private coneLift, coneBack, coconeLift, and coconeBack definitions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
PR summary 99bc1514adImport changes for modified filesNo significant changes to the import graph Import changes for all files
Declarations diff
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 Increase in tech debt: (relative, absolute) = (99.65, 2.05)
Current commit da7e40fc04 You can run this locally as
|
|
|
||
| open AddMonoidHom (flipHom coe_comp compHom flip_apply) | ||
|
|
||
| set_option backward.privateInPublic true in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this (and many others) be in the proof instead of for the whole command?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I personally think this is local enough, for the purposes of localizing the technical debt.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think like classical, debt in the types of the statement is much more worrying than debt in the proofs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the moment we have debt in the entire library. That's even more worrying.
|
Can you add comments about the non-local |
|
In fact, it seems that The same is true for |
|
No, they are needed because they need to expose something that is use in other downstream files. |
|
I think the |
|
The interesting parts of the diff look good to me. |
|
I scrolled through a bit of the mechanical diff. I trust you that it's entirely mechanical; let's land this now and refine later. Thanks for doing this! |
|
✌️ jcommelin can now approve this pull request. To approve and merge a pull request, simply reply with |
|
bors r+ |
This PR removes the global `backward.privateInPublic true` setting from the lakefile.
It adds the setting locally to many declarations. This is technical debt that will need to be cleaned later.
A few modules seem to be particularly problematic. In those the backward compatibility setting is enabled at the file level, instead of at the declaration level.
Part of the diff has been created with the help of Claude. But it struggled to do things correctly at scale and speed.
To help review, I'm giving some hints below on how to digest the diff. These might be useful in the future, so I suggest including them in the commit message.
## Review hints
### Deletions
This PR contains 14 deletions. They are contained in the following files:
```shell-session
$ git diff master --numstat | awk '$2 > 0 && $3 ~ /\.lean$/ {print $3}' | xargs git diff master --stat --
Mathlib/Algebra/Lie/TraceForm.lean | 4 ++--
Mathlib/Analysis/SpecialFunctions/Exp.lean | 4 +
Co-authored-by: Kim Morrison <kim@tqft.net>
Co-authored-by: Michael Rothgang <rothgang@math.uni-bonn.de>
|
Pull request successfully merged into master. Build succeeded: |
…unity#33034) This PR removes the global `backward.privateInPublic true` setting from the lakefile. It adds the setting locally to many declarations. This is technical debt that will need to be cleaned later. A few modules seem to be particularly problematic. In those the backward compatibility setting is enabled at the file level, instead of at the declaration level. Part of the diff has been created with the help of Claude. But it struggled to do things correctly at scale and speed. To help review, I'm giving some hints below on how to digest the diff. These might be useful in the future, so I suggest including them in the commit message. ## Review hints ### Deletions This PR contains 14 deletions. They are contained in the following files: ```shell-session $ git diff master --numstat | awk '$2 > 0 && $3 ~ /\.lean$/ {print $3}' | xargs git diff master --stat -- Mathlib/Algebra/Lie/TraceForm.lean | 4 ++-- Mathlib/Analysis/SpecialFunctions/Exp.lean | 4 + Co-authored-by: Kim Morrison <kim@tqft.net> Co-authored-by: Michael Rothgang <rothgang@math.uni-bonn.de>
This PR removes the global
backward.privateInPublic truesetting from the lakefile.It adds the setting locally to many declarations. This is technical debt that will need to be cleaned later.
A few modules seem to be particularly problematic. In those the backward compatibility setting is enabled at the file level, instead of at the declaration level.
Part of the diff has been created with the help of Claude. But it struggled to do things correctly at scale and speed.
To help review, I'm giving some hints below on how to digest the diff. These might be useful in the future, so I suggest including them in the commit message.
Review hints
Deletions
This PR contains 14 deletions. They are contained in the following files:
The
lakefile.leandeletions remove thebackward.privateInPublic*options.The other files had to be slightly refactored. Mostly
convertproofs broke in unexpected ways. I think the new proofs are better anyways.You can inspect the diff of these files with
Additions (decl-local set_options)
The bulk of this PR consists of adding
before declarations. There are 1538 of such additions.
Additions (various)
For formatting reasons, we also add some empty lines.
There are also 8 additions coming from the refactors mentioned under "Deletions" above.
I've also added two comments on decls that I think we're intended to be
privatebut were not.These are added in the files
Mathlib/CategoryTheory/Monoidal/Internal/FunctorCategory.leanMathlib/ModelTheory/PartialEquiv.leanInspect their diff with
which returns
This accounts for 1562 of the 1581 additions.
Additions (non decl-locl set_options)
The remaining 19 lines come from
set_options withoutin.These appear in the following files:
It is not entirely clear to me why these files need the more global setting. But it seems to be related to auto-generated decls that end up being private, but that are needed in other modules.