Skip to content

[Merged by Bors] - refactor(RingTheory/Ideal/Height): make Ideal.primeHeight private#37627

Closed
Thmoas-Guan wants to merge 119 commits into
leanprover-community:masterfrom
Thmoas-Guan:refactor-primeHeight
Closed

[Merged by Bors] - refactor(RingTheory/Ideal/Height): make Ideal.primeHeight private#37627
Thmoas-Guan wants to merge 119 commits into
leanprover-community:masterfrom
Thmoas-Guan:refactor-primeHeight

Conversation

@Thmoas-Guan

@Thmoas-Guan Thmoas-Guan commented Apr 4, 2026

Copy link
Copy Markdown
Collaborator

We mark Ideal.primeHeight as private, making Ideal.height the only public definition for heights of (prime) ideals. This makes the API more consistent and stops contributors from adding more declarations involving Ideal.primeHeight, which should instead be formulated in terms of Ideal.height.

To relate Ideal.height to the order theoretic Order.height in the lattice of prime ideals PrimeSpectrum we add a lemma PrimeSpectrum.height_eq_orderHeight.


Open in Gitpod

@github-actions

github-actions Bot commented Apr 4, 2026

Copy link
Copy Markdown

PR summary 45b8864a30

Import changes for modified files

No significant changes to the import graph

Import changes for all files
Files Import difference

Declarations diff

+ Ideal.height_add_one_le_of_lt_of_isPrime
+ Ideal.height_eq_inf_minimalPrimes
+ Ideal.height_eq_ringKrullDim_iff
+ Ideal.height_eq_zero_iff
+ Ideal.height_le_ringKrullDim_of_isPrime
+ Ideal.height_lt_top_of_isPrime
+ Ideal.height_mono_of_isPrime
+ Ideal.height_ne_top_of_isPrime
+ Ideal.height_strict_mono_of_isPrime
+ Ideal.height_strict_mono_of_isPrime_of_isPrime
+ Ideal.height_strict_mono_of_isPrime_of_is_prime
+ Ideal.isMaximal_of_height_eq_ringKrullDim
+ Ideal.sup_isMaximal_height_eq_ringKrullDim
+ Ideal.sup_isPrime_height_eq_ringKrullDim
+ IsLocalization.height_under_eq_of_isPrime
+ PrimeSpectrum.height_eq_orderHeight
- Ideal.height_strict_mono_of_is_prime
- IsLocalization.primeHeight_under

You can run this locally as follows
## from your `mathlib4` directory:
git clone https://github.com/leanprover-community/mathlib-ci.git ../mathlib-ci

## summary with just the declaration names:
../mathlib-ci/scripts/pr_summary/declarations_diff.sh <optional_commit>

## more verbose report:
../mathlib-ci/scripts/pr_summary/declarations_diff.sh long <optional_commit>

The doc-module for scripts/pr_summary/declarations_diff.sh in the mathlib-ci repository contains some details about this script.


No changes to strong technical debt.
No changes to weak technical debt.

@github-actions github-actions Bot added the t-ring-theory Ring theory label Apr 4, 2026
@Thmoas-Guan Thmoas-Guan added the WIP Work in progress label Apr 4, 2026
@mathlib-dependent-issues mathlib-dependent-issues Bot added the blocked-by-other-PR This PR depends on another PR (this label is automatically managed by a bot) label Apr 4, 2026
Comment thread Mathlib/RingTheory/Ideal/Height.lean Outdated
Comment thread Mathlib/RingTheory/Ideal/Height.lean Outdated
Comment thread Mathlib/RingTheory/Ideal/Height.lean Outdated

@chrisflav chrisflav left a comment

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.

Thanks a lot!
maintainer merge

@github-actions

Copy link
Copy Markdown

🚀 Pull request has been placed on the maintainer queue by chrisflav.

@mathlib-triage mathlib-triage Bot added the maintainer-merge A reviewer has approved the changed; awaiting maintainer approval. label May 21, 2026
@faenuccio

Copy link
Copy Markdown
Contributor

Can you explain why you want to make this private and update both the PR title (add a verb) and the description with this explanation?

@faenuccio faenuccio removed the maintainer-merge A reviewer has approved the changed; awaiting maintainer approval. label May 21, 2026
@faenuccio faenuccio self-assigned this May 21, 2026
@Thmoas-Guan Thmoas-Guan changed the title refactor(RingTheory/Ideal/Height): private Ideal.primeHeight refactor(RingTheory/Ideal/Height): make Ideal.primeHeight private May 22, 2026
@Thmoas-Guan

Thmoas-Guan commented May 22, 2026

Copy link
Copy Markdown
Collaborator Author

Can you explain why you want to make this private and update both the PR title (add a verb) and the description with this explanation?

The motivation is sort of vague, this part is from Andrew's old lean3 repo, but when migrating many things wasn't thought carefully enough.
The current motivation is simple, to unify exposed API thus reduce confusion. If anyone have better description about this please pin me and I'll add it in description.
The original motivation for this is suggested by @erdOne.
图片
图片

@chrisflav

Copy link
Copy Markdown
Member

Ideal.primeHeight is an implementation detail that is an auxiliary declaration to define Ideal.height. By making it private, the only public API we have is in terms of Ideal.height which makes the API more consistent. It also prevents contributors from adding new lemmas about Ideal.primeHeight that should instead be formulated in terms of Ideal.height.

@faenuccio

Copy link
Copy Markdown
Contributor

Ideal.primeHeight is an implementation detail that is an auxiliary declaration to define Ideal.height. By making it private, the only public API we have is in terms of Ideal.height which makes the API more consistent. It also prevents contributors from adding new lemmas about Ideal.primeHeight that should instead be formulated in terms of Ideal.height.

Yes, I imagined that this was the rationale. But given that the proof translating from the general height to prime height is easy but non-trivial (see

/-- For a prime ideal, its height equals its prime height. -/
lemma Ideal.height_eq_primeHeight [I.IsPrime] : I.height = I.primeHeight := by
unfold height primeHeight
simp_rw [Ideal.minimalPrimes_eq_subsingleton_self]
simp
), I wonder why you say that primeHeight is only an implementation detail. I would argue that there might be cases where people are really only working with prime ideals and don't want to go through this each time.

@chrisflav

chrisflav commented May 22, 2026

Copy link
Copy Markdown
Member

If you want to use the order theoretic height, which only makes sense for prime ideals, you can use PrimeSpectrum.height_eq_orderHeight. All downstream users of Ideal.height should not need this though, as the file Ideal.Height should already provide all required API in the general case (and if that API does not exist, we can always still add it in that file).

@faenuccio

Copy link
Copy Markdown
Contributor

If you want to use the order theoretic height, which only makes sense for prime ideals, you can use PrimeSpectrum.height_eq_orderHeight. All downstream users of Ideal.height should not need this though, as the file Ideal.Height should already provide all required API in the general case (and if that API does not exist, we can always still add it in that file).

OK, perfect, thanks, now I understand and I've reviewed the whole PR with a different eye.

@faenuccio

Copy link
Copy Markdown
Contributor

bors r+

@mathlib-triage mathlib-triage Bot added the ready-to-merge This PR has been sent to bors. label May 22, 2026
mathlib-bors Bot pushed a commit that referenced this pull request May 22, 2026
…37627)

We mark `Ideal.primeHeight` as `private`, making `Ideal.height` the only public definition for heights of (prime) ideals. This makes the API more consistent and stops contributors from adding more declarations involving `Ideal.primeHeight`, which should instead be formulated in terms of `Ideal.height`.

To relate `Ideal.height` to the order theoretic `Order.height` in the lattice of prime ideals `PrimeSpectrum` we add a lemma `PrimeSpectrum.height_eq_orderHeight`.
@mathlib-bors

mathlib-bors Bot commented May 22, 2026

Copy link
Copy Markdown
Contributor

Build failed (retrying...):

mathlib-bors Bot pushed a commit that referenced this pull request May 22, 2026
…37627)

We mark `Ideal.primeHeight` as `private`, making `Ideal.height` the only public definition for heights of (prime) ideals. This makes the API more consistent and stops contributors from adding more declarations involving `Ideal.primeHeight`, which should instead be formulated in terms of `Ideal.height`.

To relate `Ideal.height` to the order theoretic `Order.height` in the lattice of prime ideals `PrimeSpectrum` we add a lemma `PrimeSpectrum.height_eq_orderHeight`.
@mathlib-bors

mathlib-bors Bot commented May 22, 2026

Copy link
Copy Markdown
Contributor

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors Bot changed the title refactor(RingTheory/Ideal/Height): make Ideal.primeHeight private [Merged by Bors] - refactor(RingTheory/Ideal/Height): make Ideal.primeHeight private May 22, 2026
@mathlib-bors mathlib-bors Bot closed this May 22, 2026
RaggedR pushed a commit to RaggedR/mathlib4 that referenced this pull request May 24, 2026
…eanprover-community#37627)

We mark `Ideal.primeHeight` as `private`, making `Ideal.height` the only public definition for heights of (prime) ideals. This makes the API more consistent and stops contributors from adding more declarations involving `Ideal.primeHeight`, which should instead be formulated in terms of `Ideal.height`.

To relate `Ideal.height` to the order theoretic `Order.height` in the lattice of prime ideals `PrimeSpectrum` we add a lemma `PrimeSpectrum.height_eq_orderHeight`.
b-mehta pushed a commit to b-mehta/mathlib4 that referenced this pull request Jun 2, 2026
…eanprover-community#37627)

We mark `Ideal.primeHeight` as `private`, making `Ideal.height` the only public definition for heights of (prime) ideals. This makes the API more consistent and stops contributors from adding more declarations involving `Ideal.primeHeight`, which should instead be formulated in terms of `Ideal.height`.

To relate `Ideal.height` to the order theoretic `Order.height` in the lattice of prime ideals `PrimeSpectrum` we add a lemma `PrimeSpectrum.height_eq_orderHeight`.
Bergschaf pushed a commit to Bergschaf/mathlib4 that referenced this pull request Jun 3, 2026
…eanprover-community#37627)

We mark `Ideal.primeHeight` as `private`, making `Ideal.height` the only public definition for heights of (prime) ideals. This makes the API more consistent and stops contributors from adding more declarations involving `Ideal.primeHeight`, which should instead be formulated in terms of `Ideal.height`.

To relate `Ideal.height` to the order theoretic `Order.height` in the lattice of prime ideals `PrimeSpectrum` we add a lemma `PrimeSpectrum.height_eq_orderHeight`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge This PR has been sent to bors. t-ring-theory Ring theory

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants