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

chore: split long file Mathlib.Data.Set.Basic #22484

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Conversation

kim-em
Copy link
Contributor

@kim-em kim-em commented Mar 3, 2025

No description provided.

@kim-em
Copy link
Contributor Author

kim-em commented Mar 3, 2025

Will need a shake.

Copy link

github-actions bot commented Mar 3, 2025

PR summary 1ebb7babae

Import changes for modified files

No significant changes to the import graph

Import changes for all files
Files Import difference
There are 4968 files with changed transitive imports taking up over 213917 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.


Decrease in tech debt: (relative, absolute) = (1.00, 0.03)
Current number Change Type
3158 -1 porting notes
34 -1 large files

Current commit 1ebb7babae
Reference commit fa9b36533f

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).

jcommelin and others added 5 commits March 3, 2025 08:28
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Copy link
Contributor

@Vierkantor Vierkantor left a comment

Choose a reason for hiding this comment

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

I was hoping we could make some gains on the import structure: did you see a way to reduce the imports required for Data.Set.Basic, or is this pretty much the best we can do?

@leanprover-community-bot-assistant leanprover-community-bot-assistant added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Mar 3, 2025
@github-actions github-actions bot removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Mar 3, 2025
@kim-em
Copy link
Contributor Author

kim-em commented Mar 3, 2025

I was hoping we could make some gains on the import structure: did you see a way to reduce the imports required for Data.Set.Basic, or is this pretty much the best we can do?

There's not much below Data.Set.Basic:

o.pdf

We could probably get GaloisConnections.Defs and Order.Monotone out of the way? Not sure how valuable that would be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants