-
Notifications
You must be signed in to change notification settings - Fork 57
chore: simplify the connection between HasFresh and Infinite
#303
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
Conversation
ctchou
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
| fresh_notMem s := s.finite_toSet.infinite_compl.nonempty.choose_spec | ||
| This, in conjunction with `HasFresh.to_infinite`, characterizes `HasFresh`. -/ | ||
| noncomputable instance HasFresh.of_infinite (α : Type u) [Infinite α] : HasFresh α where | ||
| fresh s := Infinite.exists_notMem_finset s |>.choose |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really don't know what the preferred style is: should there be a space between |> and .?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are semantically different:
|>.chooseis projecting from the pipe (Lean.Parser.Term.pipeProj) and resolves toExists.choose|> .chooseis piping to Lean.Parser.Term.dotIdent, which cannot infer the meaning of.choosein this case becauseClassical.choosealso exists
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to know that. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case, I wonder if (Infinite.exists_notMem_finset s).choose is actually clearer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am fairly ambivalent, but I think this is a common enough core syntax to not be considered obfuscatory. I wrote it without thinking, not as a golfing attempt.
This PR adds an instance directly showing that `HasFresh` implies `Infinite`. This replaces `HasFresh.not_of_finite` which triggered a linter and is now equivalent to just `not_finite`. `HasFresh.of_infinite` is also changed to an instance with a slightly more direct definition.
…nprover#303) This PR adds an instance directly showing that `HasFresh` implies `Infinite`. This replaces `HasFresh.not_of_finite` which triggered a linter and is now equivalent to just `not_finite`. `HasFresh.of_infinite` is also changed to an instance with a slightly more direct definition.
This PR adds an instance directly showing that
HasFreshimpliesInfinite. This replacesHasFresh.not_of_finitewhich triggered a linter and is now equivalent to justnot_finite.HasFresh.of_infiniteis also changed to an instance with a slightly more direct definition.