Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[red-knot] Move
UnionBuilder
tests to Markdown #15374[red-knot] Move
UnionBuilder
tests to Markdown #15374Changes from 1 commit
bb5fee5
4eec725
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 tests would be more readable if we had something like C++'s
std::declval<T>()
(#13789) which pretends to create a value of typeT
. Usingknot_extensions
, we could now easily implement a function like the following (using PEP 747 language):that would allow us to write these tests in a single line without having to introduce dummy parameters and without having the definitions a few lines away from the assertions:
A disadvantage of this approach would be that it makes more of these tests dependent on
knot_extensions
. @AlexWaygood is still "a little sceptical" about this idea, so I kept the usual strategy of using function parameters.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.
Interestingly,
typing.cast
(with a dummy second argument), is effectively thevalue_of
function. I wouldn't be opposed to addingvalue_of
as a variant that doesn't require the dummy second argument, at the same time we addtyping.cast
. But I don't feel strongly either way, I think the function arguments are reasonable too.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.
Also, could consider
reveal_type_of_value(...)
function which has same logic ofreveal_type
that doesn't need to explicitly import?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.
More like
reveal_type_of_type_expression
or something like that, but yes — that's a good idea! We could take it one step further and implement the analog toassert_type
on that level. Basicallyassert_equal_type(type1, type2)
.And now that I write it out, I realize that we sort-of already have this in
knot_extensions
. We can writeNow that only works for fully static types, but once we have
is_gradual_equivalent_to
, that would be a (slightly more verbose) alternative toassert_equal_type
: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.
"It's like
typing.cast
, just without having a value in the first place!" is exactly how I pitched it to Alex, and he still didn't want it 😄