Skip to content

Conversation

@YaelDillies
Copy link
Collaborator

These instances are currently inferred from the IsStrictOrderedRing ones (see code snippet), but in #30563 I need these instances earlier than the IsStrictOrderedRing ones.

import Mathlib

#synth IsDomain ℕ -- IsStrictOrderedRing.isDomain
#synth IsDomain ℤ -- IsStrictOrderedRing.isDomain

Open in Gitpod

These instances are currently inferred from the `IsStrictOrderedRing` ones (see code snippet), but in leanprover-community#30563 I need these instances earlier than the `IsStrictOrderedRing` ones.

```
import Mathlib

#synth IsDomain ℕ -- IsStrictOrderedRing.isDomain
#synth IsDomain ℤ -- IsStrictOrderedRing.isDomain
```
@github-actions github-actions bot added large-import Automatically added label for PRs with a significant increase in transitive imports t-algebra Algebra (groups, rings, fields, etc) labels Oct 20, 2025
@github-actions
Copy link

PR summary 0d837987e5

Import changes exceeding 2%

% File
+7.88% Mathlib.Algebra.Ring.Int.Defs
+14.81% Mathlib.Algebra.Ring.Nat

Import changes for modified files

Dependency changes

File Base Count Head Count Change
Mathlib.Algebra.Ring.Nat 135 155 +20 (+14.81%)
Mathlib.Algebra.Ring.Int.Defs 165 178 +13 (+7.88%)
Import changes for all files
Files Import difference
8 files Mathlib.Algebra.CharZero.AddMonoidHom Mathlib.Algebra.Group.NatPowAssoc Mathlib.Algebra.Order.Ring.Unbundled.Rat Mathlib.Data.FP.Basic Mathlib.Data.Int.Bitwise Mathlib.Data.Int.ConditionallyCompleteOrder Mathlib.Data.Int.LeastGreatest Mathlib.Data.Nat.Cast.Basic
1
Mathlib.Algebra.Ring.Int.Units 8
Mathlib.Data.Int.NatAbs 11
Mathlib.Algebra.Homology.Embedding.Basic 12
Mathlib.Algebra.Ring.Int.Defs Mathlib.Data.Int.Cast.Field 13
Mathlib.Algebra.Ring.Nat 20

Declarations diff

++ instIsDomain

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.


No changes to technical debt.

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
Copy link
Member

This is causing quite an import increase in rather low-level files. Is there a different reorganization that makes sense? Possibly even a new file?

@jcommelin jcommelin added the awaiting-author A reviewer has asked the author a question or requested changes. label Oct 28, 2025
@YaelDillies
Copy link
Collaborator Author

YaelDillies commented Oct 28, 2025

Only 5 files see a significant import increase. Do you not think that's fine?

I personally think it makes a lot of sense that Algebra.Ring.Foo imports Data.Foo.Basic

@YaelDillies YaelDillies removed the awaiting-author A reviewer has asked the author a question or requested changes. label Oct 28, 2025
@kim-em
Copy link
Contributor

kim-em commented Oct 28, 2025

Yeah, let's go.

bors merge

@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the ready-to-merge This PR has been sent to bors. label Oct 28, 2025
mathlib-bors bot pushed a commit that referenced this pull request Oct 28, 2025
#30713)

These instances are currently inferred from the `IsStrictOrderedRing` ones (see code snippet), but in #30563 I need these instances earlier than the `IsStrictOrderedRing` ones.

```
import Mathlib

#synth IsDomain ℕ -- IsStrictOrderedRing.isDomain
#synth IsDomain ℤ -- IsStrictOrderedRing.isDomain
```
@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Oct 28, 2025

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title chore(Algebra/Ring): shortcut IsDomain instances for Nat and Int [Merged by Bors] - chore(Algebra/Ring): shortcut IsDomain instances for Nat and Int Oct 28, 2025
@mathlib-bors mathlib-bors bot closed this Oct 28, 2025
@YaelDillies YaelDillies deleted the domain_nat_int branch October 28, 2025 11:26
BeibeiX0 pushed a commit to BeibeiX0/mathlib4 that referenced this pull request Nov 7, 2025
leanprover-community#30713)

These instances are currently inferred from the `IsStrictOrderedRing` ones (see code snippet), but in leanprover-community#30563 I need these instances earlier than the `IsStrictOrderedRing` ones.

```
import Mathlib

#synth IsDomain ℕ -- IsStrictOrderedRing.isDomain
#synth IsDomain ℤ -- IsStrictOrderedRing.isDomain
```
FormulaRabbit81 pushed a commit to FormulaRabbit81/mathlib4 that referenced this pull request Nov 8, 2025
leanprover-community#30713)

These instances are currently inferred from the `IsStrictOrderedRing` ones (see code snippet), but in leanprover-community#30563 I need these instances earlier than the `IsStrictOrderedRing` ones.

```
import Mathlib

#synth IsDomain ℕ -- IsStrictOrderedRing.isDomain
#synth IsDomain ℤ -- IsStrictOrderedRing.isDomain
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

large-import Automatically added label for PRs with a significant increase in transitive imports ready-to-merge This PR has been sent to bors. t-algebra Algebra (groups, rings, fields, etc)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants