[WIP] Improvement of mismatching type variable error message #3911
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
As a sample of the output this now produces: are all the actual/expected parts from tests that now fail. |
|
@ilevkivskyi I have to run to catch a train, but does this look along the right lines? I'd like to include more type information about the call (i.e. what the problematic types were), but I can't work out a good way to get access to that info from where this error message is emitted. |
|
Ideally, the new error message should look like this: def func(num: int, a: AnyStr, b: AnyStr) -> AnyStr:
...
fun(1, 'a', b'b')You could try to check up the call stack and maybe pass the original argument types. If this is too hard, then at least show the numbers of the arguments involved. |
|
I did have a version of this that passed the original types through, so
I'll try that again.
(It feels like the context should be sufficient to work this sort of thing
out, but we don't have access to the type or expression checkers here,
which is what we would need.)
…On Sat, 2 Sep 2017, 18:11 Ivan Levkivskyi ***@***.***> wrote:
Ideally, the new error message should look like this:
def func(num: int, a: AnyStr, b: AnyStr) -> AnyStr:
...
fun(1, 'a', b'b')
error: Argument types for type variable "AnyStr" are incompatible in call to "func"
note: Arguments 2 and 3 for "func" must be all of the same type: (one of "str", "bytes")
note: Got:
note: type of argument 2: "str"
note: type of argument 3: "bytes"
You could try to check up the call stack and maybe pass the original
argument types. If this is too hard, then at least show the numbers of the
arguments involved.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#3911 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAD1EAx0-KWT-WQZgDhrSMrnsgUgQXm4ks5seYxbgaJpZM4PK5gU>
.
|
added 10 commits
September 3, 2017 15:59
Previously, we weren't listing arguments which had their typevar nested (e.g. List[Tuple[T, T]]).
This is the case where someone has simply passed the wrong type in for a single type variable argument.
The code path will sometimes be exercised for generic type application (and there may be other times it will be exercised that the tests don't currently cover). As the new error message only makes sense in the context of a call, only use it in that case.
ilevkivskyi
reviewed
Sep 3, 2017
| @@ -852,7 +852,8 @@ fun1(1) # E: Argument 1 to "fun1" has incompatible type "int"; expected "List[Tu | |||
| fun1([(1, 'x')]) # E: Cannot infer type argument 1 of "fun1" | |||
|
|
|||
| reveal_type(fun2([(1, 1)], 1)) # E: Revealed type is 'builtins.list[Tuple[builtins.int*, builtins.int*]]' | |||
| fun2([('x', 'x')], 'x') # E: Value of type variable "T" of "fun2" cannot be "str" | |||
| fun2([('x', 'x')], 'x') # E: Argument types for type variable "T" are incompatible in call to "fun2" \ | |||
There was a problem hiding this comment.
Taking into account that positional arguments are more often used than keyword arguments, it is much more convenient to see the position of the argument, not its name at call site.
There was a problem hiding this comment.
Understood; I'll look at making this switch as when I start passing the arguments through the call stack.
ilevkivskyi
reviewed
Sep 3, 2017
| @@ -852,7 +852,8 @@ fun1(1) # E: Argument 1 to "fun1" has incompatible type "int"; expected "List[Tu | |||
| fun1([(1, 'x')]) # E: Cannot infer type argument 1 of "fun1" | |||
|
|
|||
| reveal_type(fun2([(1, 1)], 1)) # E: Revealed type is 'builtins.list[Tuple[builtins.int*, builtins.int*]]' | |||
| fun2([('x', 'x')], 'x') # E: Value of type variable "T" of "fun2" cannot be "str" | |||
| fun2([('x', 'x')], 'x') # E: Argument types for type variable "T" are incompatible in call to "fun2" \ | |||
| # N: Arguments "v" and "scale" in call to "fun2" must all have the same type for "T" (one of "int", "bool") | |||
There was a problem hiding this comment.
In understand why you formulate the note this way (because of, e.g., T and List[T] argument types). But maybe you can show the note only if there are three or less constraints so that in this case you could propose valid variants, like:
note: Acceptable combinations of types for arguments 1 and 2 of "fun":
note: "str" and "List[str]"
note: "bytes" and "List[bytes]"
note: Got: "bytes" and "List[str]"
There was a problem hiding this comment.
This feels like it might be an enhancement on top of this change (which should end up displaying what we expected to see, and what was actually passed in).
|
@OddBloke Are you still working on this? It is an important issue. |
|
Hey Ivan,
I'm afraid I got bogged down, and I've probably lost the context I needed
to pick this up meaningfully.
Dan
…On Fri, Nov 3, 2017 at 5:53 AM Ivan Levkivskyi ***@***.***> wrote:
@OddBloke <https://github.com/oddbloke> Are you still working on this? It
is an important issue.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#3911 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAD1EHvIzOHUMPaENIReyogvVu7hbLs8ks5syuJ9gaJpZM4PK5gU>
.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This is attempting to address #963; it's WIP, and pushed up only as a basis for discussion.