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] - chore(Algebra.Module.LocalizedModule): use IsLocalization instead of Localization #9167

Closed
wants to merge 3 commits into from

Conversation

xroblot
Copy link
Collaborator

@xroblot xroblot commented Dec 20, 2023

The construction of LocalizedModule is done using IsLocalization rather than Localization. The corresponding instances for Localization are deduced automatically by Lean. One drawback is that many instances are now marked noncomputable.


Open in Gitpod

@xroblot xroblot added WIP Work in progress t-algebra Algebra (groups, rings, fields etc) labels Dec 20, 2023
@xroblot xroblot requested a review from erdOne December 21, 2023 10:03
@xroblot xroblot added awaiting-review The author would like community review of the PR and removed WIP Work in progress labels Dec 21, 2023
@erdOne
Copy link
Member

erdOne commented Dec 26, 2023

I wonder if this should be done with IsLocalizedModule?

@erdOne
Copy link
Member

erdOne commented Dec 26, 2023

In #9210 Mathlib/RingTheory/Localization/Module.lean imports Mathlib/Algebra/Module/LocalizedModule.lean instead, which IMO is the right direction. What in that file does this PR need?

@erdOne
Copy link
Member

erdOne commented Dec 26, 2023

Oh I see. We should probably find a better home for IsLocalizedModule.isBaseChange then.

@xroblot
Copy link
Collaborator Author

xroblot commented Dec 31, 2023

Oh I see. We should probably find a better home for IsLocalizedModule.isBaseChange then.

Yes, as you can see most of the heavy lifting is done in #9210. This PR is just about replacing Localization by IsLocalization.

@riccardobrasca
Copy link
Member

This is strictly more general than the current situation, right?

@xroblot
Copy link
Collaborator Author

xroblot commented Dec 31, 2023

This is strictly more general than the current situation, right?

Yes, the result about Localization can be recovered since we have the instance Localization.isLocalization. In particular, Lean can automatically infer the instance about Localization from the ones about IsLocalization.

@riccardobrasca
Copy link
Member

This is strictly more general than the current situation, right?

Yes, the result about Localization can be recovered since we have the instance Localization.isLocalization. In particular, Lean can automatically infer the instance about Localization from the ones about IsLocalization.

Thanks, let's merge this and maybe improve things later.

bors merge

@github-actions github-actions bot added ready-to-merge This PR has been sent to bors. and removed awaiting-review The author would like community review of the PR labels Dec 31, 2023
mathlib-bors bot pushed a commit that referenced this pull request Dec 31, 2023
…f `Localization` (#9167)

The construction of `LocalizedModule` is done using `IsLocalization` rather than `Localization`. The corresponding instances for `Localization` are deduced automatically by Lean. One drawback is that many instances are now marked `noncomputable`.
@mathlib-bors
Copy link

mathlib-bors bot commented Dec 31, 2023

Pull request successfully merged into master.

Build succeeded!

And happy new year! 🎉

@mathlib-bors mathlib-bors bot changed the title chore(Algebra.Module.LocalizedModule): use IsLocalization instead of Localization [Merged by Bors] - chore(Algebra.Module.LocalizedModule): use IsLocalization instead of Localization Dec 31, 2023
@mathlib-bors mathlib-bors bot closed this Dec 31, 2023
@mathlib-bors mathlib-bors bot deleted the xfr-refactor_localizedmodule branch December 31, 2023 15:43
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-algebra Algebra (groups, rings, fields etc)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants