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] - feat(data/fintype/order): complete order instances for fintype #7123

Closed
wants to merge 34 commits into from

Conversation

apnelson1
Copy link
Collaborator

@apnelson1 apnelson1 commented Apr 8, 2021

This PR adds several instances (as defs) for fintypes:

  • order_bot from semilattice_inf, order_top from semilattice_sup, bounded_order from lattice.
  • complete_lattice from lattice.
  • complete_linear_order from linear_order.

We use this last one to give a complete_linear_order instance for fin (n + 1) .


Open in Gitpod

@apnelson1 apnelson1 added the awaiting-review The author would like community review of the PR label Apr 8, 2021
@bryangingechen bryangingechen changed the title feat(data.fintype.order): complete order instances for fintype feat(data/fintype/order): complete order instances for fintype Apr 8, 2021
src/data/fintype/order.lean Outdated Show resolved Hide resolved
src/data/fintype/order.lean Outdated Show resolved Hide resolved
src/data/fintype/order.lean Outdated Show resolved Hide resolved
src/data/fintype/order.lean Outdated Show resolved Hide resolved
@apnelson1 apnelson1 force-pushed the fintype_complete_linear_order branch from 276ce6a to 3f737c7 Compare April 8, 2021 20:17
@github-actions github-actions bot added the blocked-by-other-PR This PR depends on another PR which is still in the queue. A bot manages this label via PR comment. label Apr 8, 2021
Copy link
Member

@hrmacbeth hrmacbeth left a comment

Choose a reason for hiding this comment

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

silly tweaks :)

src/data/fintype/order.lean Outdated Show resolved Hide resolved
src/data/fintype/order.lean Outdated Show resolved Hide resolved
src/data/fintype/order.lean Outdated Show resolved Hide resolved
src/data/fintype/order.lean Outdated Show resolved Hide resolved
apnelson1 and others added 4 commits April 8, 2021 16:32
Co-authored-by: hrmacbeth <25316162+hrmacbeth@users.noreply.github.com>
Co-authored-by: hrmacbeth <25316162+hrmacbeth@users.noreply.github.com>
Co-authored-by: hrmacbeth <25316162+hrmacbeth@users.noreply.github.com>
Co-authored-by: hrmacbeth <25316162+hrmacbeth@users.noreply.github.com>
src/data/fintype/order.lean Outdated Show resolved Hide resolved
@github-actions github-actions bot removed the blocked-by-other-PR This PR depends on another PR which is still in the queue. A bot manages this label via PR comment. label Apr 9, 2021
src/data/fintype/order.lean Outdated Show resolved Hide resolved
@gebner gebner 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 Apr 9, 2021
lattice -> bounded_lattice -> complete_lattice
instead of going through complete_semilattice_Sup
@apnelson1 apnelson1 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 Apr 12, 2021
@jcommelin
Copy link
Member

@apnelson1 What's the status of this PR? Do you intend to return to it in the near future? Or would you like someone else to adopt it?

@apnelson1
Copy link
Collaborator Author

@apnelson1 What's the status of this PR? Do you intend to return to it in the near future? Or would you like someone else to adopt it?

The status is 'parental leave'. I will return to work next month - I am happy to have someone else adopt the PR in the meantime if that makes sense, though.

@jcommelin
Copy link
Member

Ok, thanks for the update! Enjoy your time with your family!

@YaelDillies YaelDillies self-assigned this Nov 26, 2021
@YaelDillies YaelDillies 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 Nov 27, 2021
Copy link
Member

@eric-wieser eric-wieser left a comment

Choose a reason for hiding this comment

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

Don't all the lemmas here hold generally for any complete lattice on a fintype, not just these ones?

equality for `⊥` and `⊤`. -/
noncomputable def fintype.to_complete_linear_order [h : linear_order α] : complete_linear_order α :=
{ .. fintype.to_complete_lattice_of_lattice α,
.. h }
Copy link
Member

Choose a reason for hiding this comment

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

Could you please create two versions of this def (one with [bounded_order α] and one without)? Otherwise API for complete_lattice is inconsistent with API for complete_order.
Otherwise LGTM. Thanks!
bors d+

Copy link
Collaborator

Choose a reason for hiding this comment

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

You might want to delegate me instead. I'm not sure Peter is checking much on mathlib these days.

@bors
Copy link

bors bot commented Dec 2, 2021

✌️ apnelson1 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

urkud commented Dec 2, 2021

UPD: Probably, all these defs should be @[reducible].

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 🎉

bors merge

@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 Dec 3, 2021
bors bot pushed a commit that referenced this pull request Dec 3, 2021
This PR adds several instances (as defs) for fintypes:
* `order_bot` from `semilattice_inf`, `order_top` from `semilattice_sup`, `bounded_order` from `lattice`.
* `complete_lattice` from `lattice`.
* `complete_linear_order` from `linear_order`.

We use this last one to give a `complete_linear_order` instance for `fin (n + 1)` .



Co-authored-by: Peter Nelson <apnelson@uwaterloo.ca>
Co-authored-by: Peter Nelson <71660771+apnelson1@users.noreply.github.com>
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
Co-authored-by: YaelDillies <yael.dillies@gmail.com>
@bors
Copy link

bors bot commented Dec 3, 2021

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(data/fintype/order): complete order instances for fintype [Merged by Bors] - feat(data/fintype/order): complete order instances for fintype Dec 3, 2021
@bors bors bot closed this Dec 3, 2021
@bors bors bot deleted the fintype_complete_linear_order branch December 3, 2021 14:32
jcommelin pushed a commit that referenced this pull request Dec 18, 2021
This PR adds several instances (as defs) for fintypes:
* `order_bot` from `semilattice_inf`, `order_top` from `semilattice_sup`, `bounded_order` from `lattice`.
* `complete_lattice` from `lattice`.
* `complete_linear_order` from `linear_order`.

We use this last one to give a `complete_linear_order` instance for `fin (n + 1)` .



Co-authored-by: Peter Nelson <apnelson@uwaterloo.ca>
Co-authored-by: Peter Nelson <71660771+apnelson1@users.noreply.github.com>
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
Co-authored-by: YaelDillies <yael.dillies@gmail.com>
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

9 participants