Skip to content

[Merged by Bors] - feat: port Data.List.Lex#672

Closed
j-loreaux wants to merge 6 commits intomasterfrom
j-loreaux/data.list.lex
Closed

[Merged by Bors] - feat: port Data.List.Lex#672
j-loreaux wants to merge 6 commits intomasterfrom
j-loreaux/data.list.lex

Conversation

@j-loreaux
Copy link
Copy Markdown
Contributor

@j-loreaux j-loreaux commented Nov 21, 2022

mathlib3 SHA: 39af7d3bf61a98e928812dbc3e16f4ea8b795ca3

Porting notes:

  1. There was a mathport "failed to parenthesize" error, but it was easily worked around, see the first commit for details.
  2. I had to use aux declarations when constructing things recursively inside the field of a structure. I asked about this on Zulip and also here, but so far there doesn't seem to be a better way. In any case, it compiles so it should be fine. Otherwise, this port was very smooth.
  3. Because of the aux where (which is really let rec aux) this caused the defLemma linter to fire, which is being handled in lean4#1866, so for now we just add these declarations to nolints.json; see Zulip

@j-loreaux j-loreaux added WIP Work in progress mathlib-port This is a port of a theory file from mathlib. labels Nov 21, 2022
@j-loreaux j-loreaux added awaiting-review and removed WIP Work in progress labels Nov 21, 2022
Copy link
Copy Markdown
Member

@mcdoll mcdoll left a comment

Choose a reason for hiding this comment

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

I have the feeling that the naming of instances will be all over the place, but don't see that we have very good rules for that at the moment.

Comment on lines +204 to +206
["docBlame", "List.Lex.isOrderConnected.aux"],
["docBlame", "List.Lex.isTrichotomous.aux"],
["docBlame", "List.Lex.isAsymm.aux"],
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe instead of using nolints, just add a one-line docstring saying that it is a helper definition for the respective instance? Or do these not show up in the docs at all?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You can't add the doc strings at all. Mario fixed this in Lean core, but it hasn't trickled down yet. These will be removed from this file after the fact.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

More the point. These aren't actually defs, they're lemmas but the linter gets confused because of the let rec.

Comment on lines +2 to +4
["defLemma", "List.Lex.isTrichotomous.aux"],
["defLemma", "List.Lex.isAsymm.aux"],
["docBlame", "AddGroupWithOne"],
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It would be better if these were explicit @[nolint] attributes (with a link to the lean4 PR that will make them obsolete.

@[nolint] is for cases where there is a reason to ignore the linter. scripts/nolints.json is for embarrassing historical accidents.

@kim-em
Copy link
Copy Markdown
Contributor

kim-em commented Nov 23, 2022

bors merge

bors bot pushed a commit that referenced this pull request Nov 23, 2022
mathlib3 SHA: 39af7d3bf61a98e928812dbc3e16f4ea8b795ca3

Porting notes:
1. There was a mathport "failed to parenthesize" error, but it was easily worked around, see the first commit for details.
2. I had to use `aux` declarations when constructing things recursively inside the field of a structure. I asked about this on [Zulip](https://leanprover.zulipchat.com/#narrow/stream/287929-mathlib4/topic/recursive.20definitions.20in.20instance.20fields) and also [here](https://leanprover.zulipchat.com/#narrow/stream/270676-lean4/topic/recursive.20definition.20in.20structure.20fields), but so far there doesn't seem to be a better way. In any case, it compiles so it should be fine. Otherwise, this port was very smooth.
3. Because of the `aux where` (which is really `let rec aux`) this caused the `defLemma` linter to fire, which is being handled in [lean4#1866](leanprover/lean4#1866), so for now we just add these declarations to `nolints.json`; see [Zulip](https://leanprover.zulipchat.com/#narrow/stream/287929-mathlib4/topic/recursive.20definitions.20in.20instance.20fields)

Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
@bors
Copy link
Copy Markdown

bors bot commented Nov 23, 2022

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat: port Data.List.Lex [Merged by Bors] - feat: port Data.List.Lex Nov 23, 2022
@bors bors bot closed this Nov 23, 2022
@bors bors bot deleted the j-loreaux/data.list.lex branch November 23, 2022 13:50
bors bot pushed a commit to leanprover-community/mathlib3 that referenced this pull request Nov 23, 2022
Regenerated from the [port status wiki page](https://github.com/leanprover-community/mathlib/wiki/mathlib4-port-status).
Relates to the following PRs:
* `algebra.char_zero.defs`: leanprover-community/mathlib4#661
* `algebra.group.order_synonym`: leanprover-community/mathlib4#651
* `algebra.hierarchy_design`: leanprover-community/mathlib4#657
* `algebra.quotient`: leanprover-community/mathlib4#643
* `algebra.ring.defs`: leanprover-community/mathlib4#655
* `algebra.ring.order_synonym`: leanprover-community/mathlib4#671
* `control.equiv_functor`: leanprover-community/mathlib4#649
* `data.dlist.basic`: leanprover-community/mathlib4#616
* `data.int.cast.basic`: leanprover-community/mathlib4#670
* `data.lazy_list`: leanprover-community/mathlib4#686
* `data.list.lex`: leanprover-community/mathlib4#672
* `data.option.n_ary`: leanprover-community/mathlib4#656
* `data.sigma.lex`: leanprover-community/mathlib4#646
* `group_theory.eckmann_hilton`: leanprover-community/mathlib4#626
* `logic.equiv.basic`: leanprover-community/mathlib4#631
* `order.iterate`: leanprover-community/mathlib4#648
bors bot pushed a commit to leanprover-community/mathlib3 that referenced this pull request Nov 24, 2022
Regenerated from the [port status wiki page](https://github.com/leanprover-community/mathlib/wiki/mathlib4-port-status).
Relates to the following PRs:
* `algebra.char_zero.defs`: leanprover-community/mathlib4#661
* `algebra.group.inj_surj`: leanprover-community/mathlib4#707
* `algebra.group.order_synonym`: leanprover-community/mathlib4#651
* `algebra.group.units`: leanprover-community/mathlib4#549
* `algebra.hierarchy_design`: leanprover-community/mathlib4#657
* `algebra.quotient`: leanprover-community/mathlib4#643
* `algebra.ring.defs`: leanprover-community/mathlib4#655
* `algebra.ring.order_synonym`: leanprover-community/mathlib4#671
* `control.equiv_functor`: leanprover-community/mathlib4#649
* `data.dlist.basic`: leanprover-community/mathlib4#616
* `data.int.cast.basic`: leanprover-community/mathlib4#670
* `data.lazy_list`: leanprover-community/mathlib4#686
* `data.list.lex`: leanprover-community/mathlib4#672
* `data.option.n_ary`: leanprover-community/mathlib4#656
* `data.sigma.lex`: leanprover-community/mathlib4#646
* `data.ulift`: leanprover-community/mathlib4#703
* `group_theory.eckmann_hilton`: leanprover-community/mathlib4#626
* `logic.equiv.basic`: leanprover-community/mathlib4#631
* `order.iterate`: leanprover-community/mathlib4#648
bors bot pushed a commit to leanprover-community/mathlib3 that referenced this pull request Nov 25, 2022
Regenerated from the [port status wiki page](https://github.com/leanprover-community/mathlib/wiki/mathlib4-port-status).
Relates to the following PRs:
* `algebra.char_zero.defs`: leanprover-community/mathlib4#661
* `algebra.group.inj_surj`: leanprover-community/mathlib4#707
* `algebra.group.order_synonym`: leanprover-community/mathlib4#651
* `algebra.group.units`: leanprover-community/mathlib4#549
* `algebra.hierarchy_design`: leanprover-community/mathlib4#657
* `algebra.opposites`: leanprover-community/mathlib4#644
* `algebra.quotient`: leanprover-community/mathlib4#643
* `algebra.ring.defs`: leanprover-community/mathlib4#655
* `algebra.ring.order_synonym`: leanprover-community/mathlib4#671
* `control.equiv_functor`: leanprover-community/mathlib4#649
* `data.dlist.basic`: leanprover-community/mathlib4#616
* `data.int.cast.basic`: leanprover-community/mathlib4#670
* `data.lazy_list`: leanprover-community/mathlib4#686
* `data.list.lex`: leanprover-community/mathlib4#672
* `data.option.n_ary`: leanprover-community/mathlib4#656
* `data.sigma.lex`: leanprover-community/mathlib4#646
* `data.ulift`: leanprover-community/mathlib4#703
* `group_theory.eckmann_hilton`: leanprover-community/mathlib4#626
* `logic.equiv.basic`: leanprover-community/mathlib4#631
* `order.iterate`: leanprover-community/mathlib4#648
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mathlib-port This is a port of a theory file from mathlib.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants