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(data/finset/basic): move disjoint proofs earlier #16436

Closed
wants to merge 6 commits into from

Conversation

eric-wieser
Copy link
Member

This avoids repeating work for disj_union, and takes the stance that disjoint is part of the lattice API.


Open in Gitpod

This avoids repeating work for `disj_union`, and takes the stance that `disjoint` is part of the lattice API.
@eric-wieser eric-wieser added awaiting-review The author would like community review of the PR awaiting-CI The author would like to see what CI has to say before doing more work. t-order Order hierarchy labels Sep 8, 2022
Comment on lines +2164 to +2166
@[simp] lemma disjoint_map [decidable_eq α] [decidable_eq β] {s t : finset α} (f : α ↪ β) :
disjoint (s.map f) (t.map f) ↔ disjoint s t :=
by rw [disjoint_left, disjoint_left, ←map_disj_union_aux]
Copy link
Member Author

Choose a reason for hiding this comment

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

This lemma along with disjoint_image are the main golfs from this change; but the longer term goal is to unify the hypothesis taken by finset.disj_union with disjoint.

end bUnion

/-! ### disjoint -/
--TODO@Yaël: Kill lemmas duplicate with `boolean_algebra`
Copy link
Member Author

Choose a reason for hiding this comment

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

@YaelDillies, I assume this comment refers only to sdiff lemmas?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Absolutely.

@@ -1339,7 +1412,17 @@ sup_eq_sdiff_sup_sdiff_sup_inf
lemma erase_eq_empty_iff (s : finset α) (a : α) : s.erase a = ∅ ↔ s = ∅ ∨ s = {a} :=
by rw [←sdiff_singleton_eq_erase, sdiff_eq_empty_iff_subset, subset_singleton_iff]

end decidable_eq
--TODO@Yaël: Kill lemmas duplicate with `boolean_algebra`
Copy link
Member Author

Choose a reason for hiding this comment

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

(this comment moved along with the lemmas below)

@github-actions github-actions bot removed the awaiting-CI The author would like to see what CI has to say before doing more work. label Sep 10, 2022
src/data/finset/basic.lean Outdated Show resolved Hide resolved
src/data/finset/basic.lean Show resolved Hide resolved
src/data/finset/basic.lean Show resolved Hide resolved
Co-authored-by: Yaël Dillies <yael.dillies@gmail.com>
Copy link
Collaborator

@kmill kmill left a comment

Choose a reason for hiding this comment

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

Thanks!

bors r+

@kmill
Copy link
Collaborator

kmill commented Sep 22, 2022

bors r-

@bors
Copy link

bors bot commented Sep 22, 2022

Canceled.

@kmill
Copy link
Collaborator

kmill commented Sep 22, 2022

bors r+

@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 Sep 22, 2022
@kmill
Copy link
Collaborator

kmill commented Sep 22, 2022

Sorry, I think I didn't merge this carefully. Could you fix the merge? Then feel free to bors it.

bors r-
bors d+

@bors
Copy link

bors bot commented Sep 22, 2022

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

@bors
Copy link

bors bot commented Sep 22, 2022

Canceled.

@github-actions github-actions bot added the delegated The PR author may merge after reviewing final suggestions. label Sep 22, 2022
@kmill
Copy link
Collaborator

kmill commented Sep 22, 2022

Thanks @YaelDillies for fixing this PR. I'd have preferred if you had exercised less editorial control (i.e., not removing all the sections that Eric had introduced -- another section for symmetric differences would have sufficed), but the PR is fine as-is and I've made sure Eric is aware of your changes.

bors r+

@YaelDillies
Copy link
Collaborator

I thought those sections were remnants of your merge? Pretty sure they got deleted in another PR rather than added here.

@kmill
Copy link
Collaborator

kmill commented Sep 22, 2022

@YaelDillies All you have to do to check is go to https://github.com/leanprover-community/mathlib/pull/16436/commits and look at the commits. The sections were introduced in the first commit and never removed until 7fe139a. You can also check what my merge did, which was just deciding where to put the new stuff about symmetric differences -- the merge wasn't careful in that I didn't bother to fix the required variables. I think it's good practice to read through git histories to keep track of who is doing what and whether anyone (including myself) has accidentally deleted other people's work.

bors bot pushed a commit that referenced this pull request Sep 22, 2022
This avoids repeating work for `disj_union`, and takes the stance that `disjoint` is part of the lattice API.



Co-authored-by: Kyle Miller <kmill31415@gmail.com>
Co-authored-by: Yaël Dillies <yael.dillies@gmail.com>
@bors
Copy link

bors bot commented Sep 22, 2022

This PR was included in a batch that was canceled, it will be automatically retried

bors bot pushed a commit that referenced this pull request Sep 23, 2022
This avoids repeating work for `disj_union`, and takes the stance that `disjoint` is part of the lattice API.



Co-authored-by: Kyle Miller <kmill31415@gmail.com>
Co-authored-by: Yaël Dillies <yael.dillies@gmail.com>
@bors
Copy link

bors bot commented Sep 23, 2022

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Sep 23, 2022
This avoids repeating work for `disj_union`, and takes the stance that `disjoint` is part of the lattice API.



Co-authored-by: Kyle Miller <kmill31415@gmail.com>
Co-authored-by: Yaël Dillies <yael.dillies@gmail.com>
@bors
Copy link

bors bot commented Sep 23, 2022

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Sep 23, 2022
This avoids repeating work for `disj_union`, and takes the stance that `disjoint` is part of the lattice API.



Co-authored-by: Kyle Miller <kmill31415@gmail.com>
Co-authored-by: Yaël Dillies <yael.dillies@gmail.com>
@bors
Copy link

bors bot commented Sep 23, 2022

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Sep 23, 2022
This avoids repeating work for `disj_union`, and takes the stance that `disjoint` is part of the lattice API.



Co-authored-by: Kyle Miller <kmill31415@gmail.com>
Co-authored-by: Yaël Dillies <yael.dillies@gmail.com>
@bors
Copy link

bors bot commented Sep 23, 2022

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Sep 23, 2022
This avoids repeating work for `disj_union`, and takes the stance that `disjoint` is part of the lattice API.



Co-authored-by: Kyle Miller <kmill31415@gmail.com>
Co-authored-by: Yaël Dillies <yael.dillies@gmail.com>
@bors
Copy link

bors bot commented Sep 23, 2022

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title chore(data/finset/basic): move disjoint proofs earlier [Merged by Bors] - chore(data/finset/basic): move disjoint proofs earlier Sep 23, 2022
@bors bors bot closed this Sep 23, 2022
@bors bors bot deleted the eric-wieser/tidy-finset-disjoint branch September 23, 2022 14:45
bottine pushed a commit that referenced this pull request Sep 25, 2022
This avoids repeating work for `disj_union`, and takes the stance that `disjoint` is part of the lattice API.



Co-authored-by: Kyle Miller <kmill31415@gmail.com>
Co-authored-by: Yaël Dillies <yael.dillies@gmail.com>
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+`.) t-order Order hierarchy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants