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: prefer s ∩ . when passing to a subset of s #10433

Closed
wants to merge 3 commits into from

Conversation

kmill
Copy link
Contributor

@kmill kmill commented Feb 11, 2024

This is partial work to make s ∩ . be consistently used for passing to a subset of s. This is sort of an adjoint to (Subtype.val : s -> _) '' ., except for the fact that it does not produce a Set s.

The main API changes are to Subtype.image_preimage_val and Subtype.preimage_val_eq_preimage_val_iff in Mathlib.Data.Set.Image. Changes in other modules are all proof fixups.

Zulip discussion


Open in Gitpod

This is partial work to make `Subtype.val '' .` and `s ∩ .` be consistently used as adjoint operators for passing to and from a "subspace".
@kmill kmill added the awaiting-review The author would like community review of the PR label Feb 11, 2024
@@ -1679,7 +1679,7 @@ theorem mem_subgroupOf {H K : Subgroup G} {h : K} : h ∈ H.subgroupOf K ↔ (h

@[to_additive (attr := simp)]
theorem subgroupOf_map_subtype (H K : Subgroup G) : (H.subgroupOf K).map K.subtype = H ⊓ K :=
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Arguably this should flip too

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a TODO

@eric-wieser eric-wieser added awaiting-author A reviewer has asked the author a question or requested changes and removed awaiting-review The author would like community review of the PR labels Feb 12, 2024
@kmill kmill added awaiting-review The author would like community review of the PR and removed awaiting-author A reviewer has asked the author a question or requested changes labels Feb 13, 2024
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.

It seems to me that this convention is introducing more inter_comms than that it removes. Wouldn't it be more ergonomical to put s in the RHS slot?

@jcommelin jcommelin added awaiting-author A reviewer has asked the author a question or requested changes and removed awaiting-review The author would like community review of the PR labels Feb 14, 2024
@eric-wieser
Copy link
Member

Can you edit the description to link to the Zulip thread, @kmill?

@kmill
Copy link
Contributor Author

kmill commented Feb 14, 2024

@jcommelin I only did fixups -- inserting inter_comm is an easy way to adapt to the change.

Wouldn't it be more ergonomical to put s in the RHS slot?

Why?

@jcommelin
Copy link
Member

The diff suggests to me that you would need to do fewer fixups.

@jcommelin jcommelin added awaiting-review The author would like community review of the PR and removed awaiting-author A reviewer has asked the author a question or requested changes labels Feb 16, 2024
@jcommelin
Copy link
Member

@kmill privately explained to me that he would like "passing to a subset" to behave more like a prefix operator. Which I think is a good reason. So then this is the order it should be.

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 Feb 16, 2024
mathlib-bors bot pushed a commit that referenced this pull request Feb 16, 2024
This is partial work to make `s ∩ .` be consistently used for passing to a subset of `s`. This is sort of an adjoint to `(Subtype.val : s -> _) '' .`, except for the fact that it does not produce a `Set s`.

The main API changes are to `Subtype.image_preimage_val` and `Subtype.preimage_val_eq_preimage_val_iff` in `Mathlib.Data.Set.Image`. Changes in other modules are all proof fixups.

[Zulip discussion](https://leanprover.zulipchat.com/#narrow/stream/287929-mathlib4/topic/Passing.20to.20subsets.20of.20a.20subspace/near/420917406)
@mathlib-bors
Copy link

mathlib-bors bot commented Feb 16, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title refactor: prefer s ∩ . when passing to a subset of s [Merged by Bors] - refactor: prefer s ∩ . when passing to a subset of s Feb 16, 2024
@mathlib-bors mathlib-bors bot closed this Feb 16, 2024
@mathlib-bors mathlib-bors bot deleted the kmill_coe_inter_order branch February 16, 2024 06:30
riccardobrasca pushed a commit that referenced this pull request Feb 18, 2024
This is partial work to make `s ∩ .` be consistently used for passing to a subset of `s`. This is sort of an adjoint to `(Subtype.val : s -> _) '' .`, except for the fact that it does not produce a `Set s`.

The main API changes are to `Subtype.image_preimage_val` and `Subtype.preimage_val_eq_preimage_val_iff` in `Mathlib.Data.Set.Image`. Changes in other modules are all proof fixups.

[Zulip discussion](https://leanprover.zulipchat.com/#narrow/stream/287929-mathlib4/topic/Passing.20to.20subsets.20of.20a.20subspace/near/420917406)
miguelmarco added a commit that referenced this pull request Feb 19, 2024
dagurtomas pushed a commit that referenced this pull request Mar 22, 2024
This is partial work to make `s ∩ .` be consistently used for passing to a subset of `s`. This is sort of an adjoint to `(Subtype.val : s -> _) '' .`, except for the fact that it does not produce a `Set s`.

The main API changes are to `Subtype.image_preimage_val` and `Subtype.preimage_val_eq_preimage_val_iff` in `Mathlib.Data.Set.Image`. Changes in other modules are all proof fixups.

[Zulip discussion](https://leanprover.zulipchat.com/#narrow/stream/287929-mathlib4/topic/Passing.20to.20subsets.20of.20a.20subspace/near/420917406)
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants