Skip to content

Improve error message for an invalid callable type #4213

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
merged 1 commit into from
Nov 15, 2017

Conversation

li-dan
Copy link
Contributor

@li-dan li-dan commented Nov 6, 2017

If the user gives the wrong number of arguments to Callable[], tell them
how many arguments are expected.

@@ -812,7 +812,7 @@ class Node(Generic[T]):
def __init__(self, x: T) -> None:
...

BadC = Callable[T] # E: Invalid function type
BadC = Callable[T] # E: Callable takes 0 or 2 arguments, not 1
Copy link
Contributor

@elazarg elazarg Nov 6, 2017

Choose a reason for hiding this comment

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

This error is ambiguous, since it looks like it is about the callable itself, not the the type-constructor "Callable". At least quote it: "Callable" takes 0 or 2 arguments, not 1.

Additionally, I think it's better not to talk about "number of arguments" - again, can be confused with function arguments. Instead it might be more obvious if the message demonstrated the two possibilities. Something like Please use "Callable[[<parameters>], <return type>].

What is the "0 arguments" case? The ... form?

Copy link
Member

Choose a reason for hiding this comment

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

I guess the "0 arguments" case is plain Callable with omitted arguments. I agree with you that the message is confusing.

If the user gives the wrong number of arguments to Callable[], tell them
how many arguments are expected.
@li-dan li-dan force-pushed the invalid-callable-error branch from a498359 to 9cc1536 Compare November 6, 2017 20:42
Copy link
Collaborator

@JukkaL JukkaL left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the PR!

@JukkaL JukkaL merged commit 69a8556 into python:master Nov 15, 2017
@li-dan li-dan deleted the invalid-callable-error branch November 15, 2017 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants