Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Merged by Bors] - refactor(ring_theory): turn localization_map into a typeclass #8119

Closed
wants to merge 27 commits into from

Conversation

Vierkantor
Copy link
Collaborator

@Vierkantor Vierkantor commented Jun 29, 2021

This PR replaces the previous localization_map (M : submodule R) Rₘ definition (a ring hom R →+* Rₘ that presents Rₘ as the localization of R at M) with a new is_localization M Rₘ typeclass that puts these requirements on algebra_map R Rₘ instead. An important benefit is that we no longer need to mess with localization_map.codomain to put an R-algebra structure on Rₘ, we can just work with Rₘ directly.

The important API changes are in commit 0de78dc, all other commits are simply fixes to the dependent files.

Main changes:

  • localization_map has been replaced with is_localization, similarly away_map -> is_localization.away, localization_map.at_prime -> is_localization.at_prime and fraction_map -> is_fraction_ring
  • many declarations taking the localization_map as a parameter now take R and/or M and/or Rₘ, depending on what can be inferred easily
  • localization_map.to_map has been replaced with algebra_map R Rₘ
  • localization_map.codomain and its instances have been removed (you can now directly use Rₘ)
  • is_localization.alg_equiv generalizes fraction_map.alg_equiv_of_quotient (which has been renamed to is_fraction_ring.alg_equiv)
  • is_localization.sec has been introduced to replace (to_localization_map _ _).sec
  • localization.of have been replaced with algebra and is_localization instances on localization, similarly for localization.away.of, localization.at_prime.of and fraction_ring.of.
  • int.fraction_map is now an instance rat.is_fraction_ring
  • All files depending on the above definitions have had fixes. These were mostly straightforward, except:
  • Some category-theory arrows in algebraic_geometry/structure.sheaf are now plain ring_homs. This change was suggested by @justus-springer in order to help the elaborator figure out the arguments to is_localization.
  • Deleted minpoly.over_int_eq_over_rat and minpoly.integer_dvd, now you can just use gcd_domain_eq_field_fractions or gcd_domain_dvd respectively. This removes code duplication in minpoly.lean
  • fractional_ideal does not need to assume is_localization everywhere, only for certain specific definitions

Things that stay the same:

  • localization, localization.away, localization.at_prime and fraction_ring are still a construction of localizations (although see above for {localization,localization.away,localization.at_prime,fraction_ring}.of)

Zulip thread: https://leanprover.zulipchat.com/#narrow/stream/116395-maths/topic/Refactoring.20.60localization_map.60


Open in Gitpod

@Vierkantor Vierkantor added the WIP Work in progress label Jun 29, 2021
@Vierkantor Vierkantor added awaiting-review The author would like community review of the PR and removed WIP Work in progress labels Jun 30, 2021
@Vierkantor Vierkantor requested a review from jcommelin July 5, 2021 10:29
Copy link
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! This is looking very very good.

src/ring_theory/localization.lean Outdated Show resolved Hide resolved
From a quick grepping it seems `sec` is not used outside of this file
Copy link
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 🎉 (let's wait for CI)

bors d+

@bors
Copy link

bors bot commented Jul 5, 2021

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

@github-actions github-actions bot added delegated The PR author may merge after reviewing final suggestions. and removed awaiting-review The author would like community review of the PR labels Jul 5, 2021
@Vierkantor
Copy link
Collaborator Author

bors r+

@github-actions github-actions bot added the ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) label Jul 5, 2021
bors bot pushed a commit that referenced this pull request Jul 5, 2021
This PR replaces the previous `localization_map (M : submodule R) Rₘ` definition (a ring hom `R →+* Rₘ` that presents `Rₘ` as the localization of `R` at `M`) with a new `is_localization M Rₘ` typeclass that puts these requirements on `algebra_map R Rₘ` instead. An important benefit is that we no longer need to mess with `localization_map.codomain` to put an `R`-algebra structure on `Rₘ`, we can just work with `Rₘ` directly.

The important API changes are in commit 0de78dc, all other commits are simply fixes to the dependent files.

Main changes:
 * `localization_map` has been replaced with `is_localization`, similarly `away_map` -> `is_localization.away`, `localization_map.at_prime` -> `is_localization.at_prime` and `fraction_map` -> `is_fraction_ring`
 * many declarations taking the `localization_map` as a parameter now take `R` and/or `M` and/or `Rₘ`, depending on what can be inferred easily
 * `localization_map.to_map` has been replaced with `algebra_map R Rₘ`
 * `localization_map.codomain` and its instances have been removed (you can now directly use `Rₘ`)
 * `is_localization.alg_equiv` generalizes `fraction_map.alg_equiv_of_quotient` (which has been renamed to `is_fraction_ring.alg_equiv`)
 * `is_localization.sec` has been introduced to replace `(to_localization_map _ _).sec`
 * `localization.of` have been replaced with `algebra` and `is_localization` instances on `localization`, similarly for `localization.away.of`, `localization.at_prime.of` and `fraction_ring.of`.
 * `int.fraction_map` is now an instance `rat.is_fraction_ring`
 * All files depending on the above definitions have had fixes. These were mostly straightforward, except:
 * [Some category-theory arrows in `algebraic_geometry/structure.sheaf` are now plain `ring_hom`s. This change was suggested by @justus-springer in order to help the elaborator figure out the arguments to  `is_localization`.](cf3acc9)
 * Deleted `minpoly.over_int_eq_over_rat` and `minpoly.integer_dvd`, now you can just use `gcd_domain_eq_field_fractions` or `gcd_domain_dvd` respectively. [This removes code duplication in `minpoly.lean`](5695924)
 * `fractional_ideal` does not need to assume `is_localization` everywhere, only for certain specific definitions

Things that stay the same:
 * `localization`, `localization.away`, `localization.at_prime` and `fraction_ring` are still a construction of localizations (although see above for `{localization,localization.away,localization.at_prime,fraction_ring}.of`)

Zulip thread: https://leanprover.zulipchat.com/#narrow/stream/116395-maths/topic/Refactoring.20.60localization_map.60
@bors
Copy link

bors bot commented Jul 5, 2021

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title refactor(ring_theory): turn localization_map into a typeclass [Merged by Bors] - refactor(ring_theory): turn localization_map into a typeclass Jul 5, 2021
@bors bors bot closed this Jul 5, 2021
@bors bors bot deleted the localization_typeclass branch July 5, 2021 14:57
b-mehta pushed a commit that referenced this pull request Jul 6, 2021
This PR replaces the previous `localization_map (M : submodule R) Rₘ` definition (a ring hom `R →+* Rₘ` that presents `Rₘ` as the localization of `R` at `M`) with a new `is_localization M Rₘ` typeclass that puts these requirements on `algebra_map R Rₘ` instead. An important benefit is that we no longer need to mess with `localization_map.codomain` to put an `R`-algebra structure on `Rₘ`, we can just work with `Rₘ` directly.

The important API changes are in commit 0de78dc, all other commits are simply fixes to the dependent files.

Main changes:
 * `localization_map` has been replaced with `is_localization`, similarly `away_map` -> `is_localization.away`, `localization_map.at_prime` -> `is_localization.at_prime` and `fraction_map` -> `is_fraction_ring`
 * many declarations taking the `localization_map` as a parameter now take `R` and/or `M` and/or `Rₘ`, depending on what can be inferred easily
 * `localization_map.to_map` has been replaced with `algebra_map R Rₘ`
 * `localization_map.codomain` and its instances have been removed (you can now directly use `Rₘ`)
 * `is_localization.alg_equiv` generalizes `fraction_map.alg_equiv_of_quotient` (which has been renamed to `is_fraction_ring.alg_equiv`)
 * `is_localization.sec` has been introduced to replace `(to_localization_map _ _).sec`
 * `localization.of` have been replaced with `algebra` and `is_localization` instances on `localization`, similarly for `localization.away.of`, `localization.at_prime.of` and `fraction_ring.of`.
 * `int.fraction_map` is now an instance `rat.is_fraction_ring`
 * All files depending on the above definitions have had fixes. These were mostly straightforward, except:
 * [Some category-theory arrows in `algebraic_geometry/structure.sheaf` are now plain `ring_hom`s. This change was suggested by @justus-springer in order to help the elaborator figure out the arguments to  `is_localization`.](cf3acc9)
 * Deleted `minpoly.over_int_eq_over_rat` and `minpoly.integer_dvd`, now you can just use `gcd_domain_eq_field_fractions` or `gcd_domain_dvd` respectively. [This removes code duplication in `minpoly.lean`](5695924)
 * `fractional_ideal` does not need to assume `is_localization` everywhere, only for certain specific definitions

Things that stay the same:
 * `localization`, `localization.away`, `localization.at_prime` and `fraction_ring` are still a construction of localizations (although see above for `{localization,localization.away,localization.at_prime,fraction_ring}.of`)

Zulip thread: https://leanprover.zulipchat.com/#narrow/stream/116395-maths/topic/Refactoring.20.60localization_map.60
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
delegated The PR author may merge after reviewing final suggestions. ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants