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(data/set/finite): change type of set.finite.dependent_image #6475

Closed
wants to merge 2 commits into from

Conversation

urkud
Copy link
Member

@urkud urkud commented Feb 28, 2021

The old lemma combined a statement similar to set.finite.image with
set.finite.subset. The new statement is a direct generalization of
set.finite.image.


The old lemma combined a statement similar to `set.finite.image` with
`set.finite.subset`. The new statement is a direct generalization of
`set.finite.image`.
@urkud urkud added the awaiting-review The author would like community review of the PR label Feb 28, 2021
@urkud urkud requested a review from sgouezel February 28, 2021 05:57
lemma finite.dependent_image {s : set α} (hs : finite s) {F : Π i ∈ s, β} {t : set β}
(H : ∀ y ∈ t, ∃ x (hx : x ∈ s), y = F x hx) : set.finite t :=
lemma finite.dependent_image {s : set α} (hs : finite s) (F : Π i ∈ s, β) :
finite {y : β | ∃ x (hx : x ∈ s), F x hx = y} :=
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why did you change the order here? I mean, go from y = F x hx to F x hx = y? Typically, in applications if you rcase along this you will get an assumption h : y = F x hx along which you can rewrite directly, while if you get F x hx = y you will have to rewrite in the reverse direction, which seems less natural.

Copy link
Member Author

Choose a reason for hiding this comment

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

My goal was to match the definition of set.image but I can revert this part (or drop the whole PR - it turns out that I won't need it).

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't mind the PR, but I really prefer the other direction.

@@ -573,7 +573,7 @@ def comp_partial_sum_target_set (m M N : ℕ) : set (Σ n, composition n) :=

lemma comp_partial_sum_target_subset_image_comp_partial_sum_source
(m M N : ℕ) (i : Σ n, composition n) (hi : i ∈ comp_partial_sum_target_set m M N) :
∃ j (hj : j ∈ comp_partial_sum_source m M N), i = comp_change_of_variables m M N j hj :=
∃ j (hj : j ∈ comp_partial_sum_source m M N), comp_change_of_variables m M N j hj = i :=
Copy link
Member

Choose a reason for hiding this comment

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

Reversign the direction here seems similarly undesirable, as obtain \<j, hj, rfl\> := comp_partial_sum_target_subset_image_comp_partial_sum_source _ _ _ i _, no longer works.

@sgouezel
Copy link
Collaborator

sgouezel commented Mar 2, 2021

I have changed back the direction. If you are happy with this, you can merge yourself.
bors d+

@bors
Copy link

bors bot commented Mar 2, 2021

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

@urkud
Copy link
Member Author

urkud commented Mar 2, 2021

bors merge

@urkud
Copy link
Member Author

urkud commented Mar 2, 2021

Thanks!

@github-actions github-actions bot added ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) and removed awaiting-review The author would like community review of the PR labels Mar 2, 2021
bors bot pushed a commit that referenced this pull request Mar 2, 2021
#6475)

The old lemma combined a statement similar to `set.finite.image` with
`set.finite.subset`. The new statement is a direct generalization of
`set.finite.image`.



Co-authored-by: sgouezel <sebastien.gouezel@univ-rennes1.fr>
@bors
Copy link

bors bot commented Mar 2, 2021

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title refactor(data/set/finite): change type of set.finite.dependent_image [Merged by Bors] - refactor(data/set/finite): change type of set.finite.dependent_image Mar 2, 2021
@bors bors bot closed this Mar 2, 2021
@bors bors bot deleted the fin-dep-img branch March 2, 2021 22:07
b-mehta pushed a commit that referenced this pull request Apr 2, 2021
#6475)

The old lemma combined a statement similar to `set.finite.image` with
`set.finite.subset`. The new statement is a direct generalization of
`set.finite.image`.



Co-authored-by: sgouezel <sebastien.gouezel@univ-rennes1.fr>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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