Skip to content

chore(Topology/PseudoMetric): split file#14048

Closed
kim-em wants to merge 3 commits intomasterfrom
split_pseudometric
Closed

chore(Topology/PseudoMetric): split file#14048
kim-em wants to merge 3 commits intomasterfrom
split_pseudometric

Conversation

@kim-em
Copy link
Copy Markdown
Contributor

@kim-em kim-em commented Jun 23, 2024

No description provided.

@kim-em kim-em added WIP Work in progress awaiting-CI This PR does not pass CI yet. This label is automatically removed once it does. labels Jun 23, 2024
@github-actions
Copy link
Copy Markdown

github-actions bot commented Jun 23, 2024

PR summary e9dc166942

Import changes

Dependency changes

File Base Count Head Count Change
Mathlib.Topology.MetricSpace.Cauchy 830 827 -3 (-0.36%)
Mathlib.Topology.MetricSpace.Equicontinuity 833 830 -3 (-0.36%)
Mathlib.Topology.MetricSpace.Basic 830 829 -1 (-0.12%)
Mathlib.Topology.Metrizable.Basic 831 830 -1 (-0.12%)
Mathlib.Analysis.Convex.Extrema 1004 1003 -1 (-0.10%)

Declarations diff

--++ -/

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

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

@grunweg
Copy link
Copy Markdown
Contributor

grunweg commented Jun 23, 2024

Let me cross-reference #13977 which tries the same. (I'll be happy to review either, but submitting my PhD thesis takes priority -> I may take about a week to get to this.)

@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 Jun 23, 2024
@YaelDillies
Copy link
Copy Markdown
Contributor

@adomani, a clearly inaccurate declaration diff here!

@adomani
Copy link
Copy Markdown
Contributor

adomani commented Jun 23, 2024

I think that the output that you are seeing comes from this:

$ git diff --unified=0 master...HEAD | grep "structure. -/"
-structure. -/
-structure. -/
+structure. -/
+structure. -/

so, not ideal, but also not worrisome!

It could probably be fixed by adding an extra space after structure to be matched in the regex that identifies these, but I am not sure that this would not cause other problems later on.

See here and a couple of lines below.

@adomani
Copy link
Copy Markdown
Contributor

adomani commented Jun 23, 2024

I confirm: if you change

  awk -v regex="^[+-]${begs}" 'BEGIN{ paired=0; added=0; removed=0 }

to

  awk -v regex="^[+-]${begs} " 'BEGIN{ paired=0; added=0; removed=0 }

in no_lost_declarations, then here it reports no change.

@kim-em kim-em closed this Jun 23, 2024
@YaelDillies
Copy link
Copy Markdown
Contributor

@adomani please tell me when you open a PR to fix the regex! 🙏

mathlib-bors bot pushed a commit that referenced this pull request Jun 24, 2024
As [reported](#14048 (comment)) and [requested](#14048 (comment)) by Yael!

I think that this change is an improvement, but I would be happier if there was a consensus that the `def`, `theorem`, `structure`,... keywords are followed by a space and not a line break before their identifier.
kbuzzard pushed a commit that referenced this pull request Jun 26, 2024
As [reported](#14048 (comment)) and [requested](#14048 (comment)) by Yael!

I think that this change is an improvement, but I would be happier if there was a consensus that the `def`, `theorem`, `structure`,... keywords are followed by a space and not a line break before their identifier.
dagurtomas pushed a commit that referenced this pull request Jul 2, 2024
As [reported](#14048 (comment)) and [requested](#14048 (comment)) by Yael!

I think that this change is an improvement, but I would be happier if there was a consensus that the `def`, `theorem`, `structure`,... keywords are followed by a space and not a line break before their identifier.
@YaelDillies YaelDillies deleted the split_pseudometric branch August 17, 2025 11:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

WIP Work in progress

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants