8000 Prohibit Callable[[...], X] by ilevkivskyi · Pull Request #320 · python/typing · GitHub
[go: up one dir, main page]
Skip to content

Prohibit Callable[[...], X] #320

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.

Sign up for GitHub

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 6 commits into from
Nov 7, 2016
Merged

Prohibit Callable[[...], X] #320

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
39e9a4b
Prohibit Callable[[...], X]
ilevkivskyi Nov 6, 2016
f2d9c14
A simpler (and probably right) fix
Nov 7, 2016
9e2ada7
Additional cleanup as suggested
Nov 7, 2016
5c5fb0a
The same for PY2
Nov 7, 2016
9f6a0eb
One more bit for PY2
Nov 7, 2016
ea1ed26
Implemented latest comments
Nov 7, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Prohibit Callable[[...], X]
  • Loading branch information
@ilevkivskyi
ilevkivskyi committed Nov 6, 2016
commit 39e9a4be1cdb8fde041efeef3321c679f1097065
8 changes: 8 additions & 0 deletions python2/test_typing.py
View file
Open in desktop
10000
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,14 @@ def test_cannot_instantiate(self):
with self.assertRaises(TypeError):
type(c)()

def test_callable_wrong_forms(self):
with self.assertRaises(TypeError):
Callable[(), int]
with self.assertRaises(TypeError):
Callable[[()], int]
with self.assertRaises(TypeError):
Callable[[int, 1], 2]

def test_callable_instance_works(self):
def f():
pass
Expand Down
6 changes: 3 additions & 3 deletions python2/typing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1308,9 +1308,9 @@ def __getitem__(self, parameters):
elif args == []:
parameters = ((), result)
else:
if not isinstance(args, list):
raise TypeError("Callable[args, result]: args must be a list."
" Got %.100r." % (args,))
if not isinstance(args, list) or Ellipsis in args or () in args:
raise TypeError("Callable[args, result]: args must be"
" a list of types. Got %.100r." % (args,))
parameters = tuple(args) + (result,)
return self.__getitem_inner__(parameters)

Expand Down
10 changes: 10 additions & 0 deletions src/test_typing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,16 @@ def test_cannot_instantiate(self):
with self.assertRaises(TypeError):
type(c)()

def test_callable_wrong_forms(self):
with self.assertRaises(TypeError):
Callable[[...], int]
with self.assertRaises(TypeError):
Callable[(), int]
with self.assertRaises(TypeError):
Callable[[()], int]
with self.assertRaises(TypeError):
Callable[[int, 1], 2]

def test_callable_instance_works(self):
def f():
pass
Expand Down
6 changes: 3 additions & 3 deletions src/typing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1221,9 +1221,9 @@ def __getitem__(self, parameters):
elif args == []:
Copy link
Member

Choose a reason for hiding this comment

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

I don't think you need this special case, and if you remove it and the matching special case in __getitem_inner__ the tests still passes. (It's different here than for Tuple -- while Tuple[] is invalid due to Python's syntax, Callable[[], t] is fine, and there's no need to treat an empty list special.

PS. I can't add a comment there, but your code below has [...,] and [(),] -- those trailing commas are not needed.

Copy link
Member Author

Choose a reason for hiding this comment

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

@gvanrossum You are right, only ellipsis needs special case (in two places you mention, and also in __repr__).

parameters = ((), result)
else:
if not isinstance(args, list):
raise TypeError("Callable[args, result]: args must be a list."
" Got %.100r." % (args,))
if not isinstance(args, list) or ... in args or () in args:
Copy link
Member

Choose a reason for hiding this comment

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

Hm... It really shouldn't be necessary to check this here. I think the reason may be that you're trying to make the hash key for e.g. Callable[[int, str], float] be the tuple (int, str, float) rather than just nesting the original structure. The exceptions in __getitem_inner__ seem due to this.

Or maybe you're trying to make __parameters__ a single-level tuple? But there are two exceptions for this, one for Callable[..., t] and one for Callable[[], t] (IIUC). I don't think this is really worth it...

Copy link
Member Author

Choose a reason for hiding this comment

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

@gvanrossum Yes, indeed I wanted to flatten __parameters__ to just call super().__getitem__. But you are right, it is not necessary to do this here, but only right before I actually call super().

raise TypeError("Callable[args, result]: args must be"
" a list of types. Got %.100r." % (args,))
parameters = tuple(args) + (result,)
return self.__getitem_inner__(parameters)

Expand Down
0