Skip to content

feat: the metrisableSpace linter#17519

Closed
grunweg wants to merge 285 commits intomasterfrom
MR-metrisable-lint
Closed

feat: the metrisableSpace linter#17519
grunweg wants to merge 285 commits intomasterfrom
MR-metrisable-lint

Conversation

@grunweg
Copy link
Copy Markdown
Contributor

@grunweg grunweg commented Oct 7, 2024

Similar to #10235, lint on occurrences of MetricSpace which should be MetrisableSpace.


Open in Gitpod

@mathlib4-dependent-issues-bot mathlib4-dependent-issues-bot added the blocked-by-other-PR This PR depends on another PR (this label is automatically managed by a bot) label Oct 7, 2024
@mathlib4-dependent-issues-bot
Copy link
Copy Markdown
Collaborator

This PR/issue depends on:

@grunweg grunweg force-pushed the MR-metrisable-lint branch from 5be918f to 3089ff0 Compare October 7, 2024 21:02
@github-actions
Copy link
Copy Markdown

github-actions bot commented Oct 7, 2024

PR summary 3089ff0141

Import changes for modified files

No significant changes to the import graph

Import changes for all files
Files Import difference
There are 4613 files with changed transitive imports taking up over 191993 characters: this is too many to display!
You can run scripts/import_trans_difference.sh all locally to see the whole output.

Declarations diff

+ checkUnusedAssumptionInType
+ decidableClassical
+ encodableCountable
+ finiteFintype
+ inhabitedNonempty
+ metricMetrisable
+ partitionPoint

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.

but don't use this assumption in the type.
(Instead, `(Pseudo)MetrisableSpace X` can suffice, or the assumption can be fully removed.)
-/
@[env_linter] def metricMetrisable : Linter where
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.

Note that we spell it as Metrizable in TopologicalSpace.MetrizableSpace

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.

My guess is that this linter doesn't catch anything, because normally you use data in [MetricSpace X], otherwise this would be an unused argument. The linter should catch theorems that only use the projection to UniformSpace. Another question is how to catch uses of [UniformSpace X] that only use projection to TopologicalSpace X (should be replaced by [TopologicalSpace X] [CompletelyRegularSpace X] once we redefine the latter to avoid dependency on Real) or [UniformSpace X] [Filter.IsCountablyGenerated (uniformity X)] that only use the projection to TopologicalSpace X everywhere else (should be replaced by [PseudoMetrizableSpace X].

mathlib-bors bot pushed a commit that referenced this pull request Oct 14, 2024
…e` (#17520)

Discovered through the linter in #17519 (but in fact not related to it).



Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
Co-authored-by: Michael Rothgang <rothgang@math.uni-bonn.de>
@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 Oct 16, 2024
@grunweg grunweg added the t-linter Linter label Oct 16, 2024
@grunweg
Copy link
Copy Markdown
Contributor Author

grunweg commented Feb 15, 2026

Closing in favour of #35340.

@grunweg grunweg closed this Feb 15, 2026
@YaelDillies YaelDillies deleted the MR-metrisable-lint branch March 29, 2026 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

blocked-by-other-PR This PR depends on another PR (this label is automatically managed by a bot) merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) t-linter Linter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants