Skip to content

[Merged by Bors] - chore: change the definition of List.finRange#19447

Closed
kim-em wants to merge 11 commits intomasterfrom
finRange_def
Closed

[Merged by Bors] - chore: change the definition of List.finRange#19447
kim-em wants to merge 11 commits intomasterfrom
finRange_def

Conversation

@kim-em
Copy link
Copy Markdown
Contributor

@kim-em kim-em commented Nov 25, 2024

François Dorais has been working on upstreaming List.finRange, but wants to change the definition at the same time. That was running into difficulties, which hopefully are resolved here.

@github-actions github-actions bot added the large-import Automatically added label for PRs with a significant increase in transitive imports label Nov 25, 2024
@github-actions
Copy link
Copy Markdown

github-actions bot commented Nov 25, 2024

PR summary c25fea8a05

Import changes exceeding 2%

% File
+2.61% Mathlib.Data.List.Sublists

Import changes for modified files

Dependency changes

File Base Count Head Count Change
Mathlib.Data.List.Range 339 287 -52 (-15.34%)
Mathlib.Data.List.Sublists 345 354 +9 (+2.61%)
Mathlib.Data.List.FinRange 351 348 -3 (-0.85%)
Import changes for all files
Files Import difference
Mathlib.Data.List.Range -52
Mathlib.Data.List.FinRange -3
Mathlib.Data.Finset.Powerset 5
Mathlib.Algebra.BigOperators.Ring.Multiset 8
3 files Mathlib.Data.Multiset.Antidiagonal Mathlib.Data.List.Sublists Mathlib.Data.Multiset.Powerset
9

Declarations diff

+ finRange_eq_pmap_range

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) = (1.00, 0.01)
Current number Change Type
184 1 adaptation notes

Current commit c25fea8a05
Reference commit b96544419e

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


/- #adaptation_note: this attribute should be removed after Mathlib moves to v4.15.0-rc1. -/
set_option allowUnsafeReducibility true in
attribute [semireducible] Fin.foldr.loop
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Does this apply globally, or just for the current file?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I also worry that this PR might have accumulated a bunch of proof uglification before we discovered this fix; is that the case, or do I have the chronology wrong?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The uglification seems to still be necessary at the moment, and I'm not really sure why it is happening. It is only at the level of having to redo some simp only blocks, however.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Does this apply globally, or just for the current file?

This is global.

@github-actions github-actions bot removed the large-import Automatically added label for PRs with a significant increase in transitive imports label Nov 25, 2024
@github-actions github-actions bot added the large-import Automatically added label for PRs with a significant increase in transitive imports label Nov 25, 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 🎉

If CI passes, please remove the label awaiting-CI and merge this yourself, by adding a comment bors r+.

bors d+

@mathlib-bors
Copy link
Copy Markdown
Contributor

mathlib-bors bot commented Nov 25, 2024

✌️ kim-em can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@ghost ghost added the delegated This pull request has been delegated to the PR author (or occasionally another non-maintainer). label Nov 25, 2024
@jcommelin jcommelin added the awaiting-CI This PR does not pass CI yet. This label is automatically removed once it does. label Nov 25, 2024
@github-actions github-actions bot removed the awaiting-CI This PR does not pass CI yet. This label is automatically removed once it does. label Nov 25, 2024
@kim-em
Copy link
Copy Markdown
Contributor Author

kim-em commented Nov 25, 2024

bors merge

@ghost ghost added the ready-to-merge This PR has been sent to bors. label Nov 25, 2024
mathlib-bors bot pushed a commit that referenced this pull request Nov 25, 2024
François Dorais has been [working](leanprover-community/batteries#1055) on upstreaming `List.finRange`, but wants to change the definition at the same time. That was running into difficulties, which hopefully are resolved here.
@mathlib-bors
Copy link
Copy Markdown
Contributor

mathlib-bors bot commented Nov 25, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title chore: change the definition of List.finRange [Merged by Bors] - chore: change the definition of List.finRange Nov 25, 2024
@mathlib-bors mathlib-bors bot closed this Nov 25, 2024
@mathlib-bors mathlib-bors bot deleted the finRange_def branch November 25, 2024 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

delegated This pull request has been delegated to the PR author (or occasionally another non-maintainer). large-import Automatically added label for PRs with a significant increase in transitive imports 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