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/nat/lattice): move code, add lemmas #8708

Closed
wants to merge 4 commits into from

Conversation

urkud
Copy link
Member

@urkud urkud commented Aug 16, 2021

  • move nat.conditionally_complete_linear_order_with_bot and enat.complete_linear_order to a new file data.nat.lattice;
  • add a few lemmas (nat.supr_lt_succ etc), move set.bUnion_lt_succ to the same file;
  • use galois_insertion.lift_complete_lattice to define enat.complete_linear_order.

Open in Gitpod

@urkud urkud added the awaiting-review The author would like community review of the PR label Aug 16, 2021
namespace enat
open_locale classical

noncomputable instance : complete_linear_order enat :=
Copy link
Member

Choose a reason for hiding this comment

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

Does it make more sense to put this in data.nat.enat and change the imports a bit? Or does that not work?

Copy link
Member Author

Choose a reason for hiding this comment

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

I have no opinion here. Actually, I wanted to add nat.supr_lt_succ etc and there was no good home for these lemmas, so I moved code around. If you want me to make data.nat.enat import data.nat.lattice, then I can do it. OTOH, having all *nat (conditionally) complete lattice instances in one file makes sense too.

Copy link
Member

Choose a reason for hiding this comment

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

I also don't care and don't think it matters much, just caught my eye that this was the only mention of enat in the new file and it came at the very end. Up to you.

Comment on lines +87 to +89
/-- This instance is necessary, otherwise the lattice operations would be derived via
conditionally_complete_linear_order_bot and marked as noncomputable. -/
instance : lattice ℕ := lattice_of_linear_order
Copy link
Member

Choose a reason for hiding this comment

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

Should this go here?

instance nat.distrib_lattice : distrib_lattice ℕ :=
by apply_instance

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't want to move more instances from files deeper in the import chain without a reason.

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 d+

@bors
Copy link

bors bot commented Aug 20, 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.

@github-actions github-actions bot added delegated The PR author may merge after reviewing final suggestions. and removed awaiting-review The author would like community review of the PR labels Aug 20, 2021
@urkud
Copy link
Member Author

urkud commented Aug 20, 2021

bors merge

@github-actions github-actions bot added the ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) label Aug 20, 2021
bors bot pushed a commit that referenced this pull request Aug 20, 2021
* move `nat.conditionally_complete_linear_order_with_bot` and `enat.complete_linear_order` to a new file `data.nat.lattice`;
* add a few lemmas (`nat.supr_lt_succ` etc), move `set.bUnion_lt_succ` to the same file;
* use `galois_insertion.lift_complete_lattice` to define `enat.complete_linear_order`.
@bors
Copy link

bors bot commented Aug 20, 2021

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title refactor(data/nat/lattice): move code, add lemmas [Merged by Bors] - refactor(data/nat/lattice): move code, add lemmas Aug 20, 2021
@bors bors bot closed this Aug 20, 2021
@bors bors bot deleted the nat-lattice branch August 20, 2021 23:33
/-!
# Conditionally complete linear order structure on `ℕ`

In this file we
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can somebody fix this broken docstring?

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+`.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants