Skip to content

bpo-42195: Ensure consistency of Callable's __args__ in collections.abc and typing#23060

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
gvanrossum merged 31 commits into from
Dec 13, 2020
Merged

bpo-42195: Ensure consistency of Callable's __args__ in collections.abc and typing #23060

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
2c4a297
Allow subclassing of GenericAlias, fix collections.abc.Callable's Gen…
Fidget-Spinner Oct 31, 2020
588d421
fix typing tests, add hash and eq methods
Fidget-Spinner Oct 31, 2020
050fa13
Fix pickling
Fidget-Spinner Oct 31, 2020
f60ea8a
whitespace
Fidget-Spinner Oct 31, 2020
2f3c6dc
Add test specifically for bpo
Fidget-Spinner Oct 31, 2020
2c4508e
update error message
Fidget-Spinner Oct 31, 2020
19d2973
Appease test_site
Fidget-Spinner Oct 31, 2020
3116c8e
Represent Callable __args__ via [tuple[args], result]
Fidget-Spinner Nov 19, 2020
f2b593a
add back tests for weakref, styling nits, add news
Fidget-Spinner Nov 19, 2020
93d51e4
remove redundant tuple checks leftover from old code
Fidget-Spinner Nov 19, 2020
327e1a5
Use _PosArgs instead of tuple
Fidget-Spinner Nov 28, 2020
e971ccb
Fix typo and news
Fidget-Spinner Nov 29, 2020
abd8b98
Refactor C code to use less duplication
Fidget-Spinner Nov 30, 2020
3ddca06
Address most of Guido's reviews (tests failing on purpose)
Fidget-Spinner Dec 1, 2020
1ab59c5
try to revert back to good old flat tuple __args__ days
Fidget-Spinner Dec 2, 2020
ee2d2e1
getting even closer
Fidget-Spinner Dec 2, 2020
2015738
finally done
Fidget-Spinner Dec 2, 2020
6704ffd
Update news
Fidget-Spinner Dec 4, 2020
598d29b
Address review partially
Fidget-Spinner Dec 5, 2020
c43ebcf
Address review fully, update news and tests, remove try-except block
Fidget-Spinner Dec 5, 2020
37ae3a9
Borrowed references don't need decref
Fidget-Spinner Dec 5, 2020
adbfcad
improve _PyArg_NoKwnames error handling, add union and subclass tests
Fidget-Spinner Dec 5, 2020
d1dd627
Don't change getargs, use _PyArg_NoKeywords instead
Fidget-Spinner Dec 5, 2020
2c21045
Merge remote-tracking branch 'upstream/master' into abc-callable-ga
Fidget-Spinner Dec 5, 2020
9f71667
remove stray whitespace
Fidget-Spinner Dec 5, 2020
a789620
refactor C code, add deprecation warning for 3.9
Fidget-Spinner Dec 6, 2020
1890b37
remove redundant check in C code, and try except in __new__
Fidget-Spinner Dec 6, 2020
4e928c6
remove check
Fidget-Spinner Dec 7, 2020
6b11d33
Loosen type checks for Callable args, cast to PyObject in genericalia…
Fidget-Spinner Dec 11, 2020
4215c3b
update news to mention about removing validation in argtypes
Fidget-Spinner Dec 11, 2020
585bf19
remove commented out code
Fidget-Spinner Dec 12, 2020
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
69 changes: 68 additions & 1 deletion Lib/_collections_abc.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,6 +10,10 @@
import sys

GenericAlias = type(list[int])
EllipsisType = type(...)
def _f(): pass
FunctionType = type(_f)
del _f

__all__ = ["Awaitable", "Coroutine",
"AsyncIterable", "AsyncIterator", "AsyncGenerator",
Expand DownExpand Up@@ -409,6 +413,69 @@ def __subclasshook__(cls, C):
return NotImplemented


class _CallableGenericAlias(GenericAlias):
""" Represent `Callable[argtypes, resulttype]`.

This sets ``__args__`` to a tuple containing the flattened``argtypes``
followed by ``resulttype``.

Example: ``Callable[[int, str], float]`` sets ``__args__`` to
``(int, str, float)``.
"""

__slots__ = ()

def __new__(cls, origin, args):
Copy link
Member

@gvanrossumgvanrossumDec 5, 2020

Choose a reason for hiding this comment

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

Okay, for 3.10 I think this is the right solution.

For 3.9, I think we need to issue a DeprecationWarning(using the warnings module) with the same message (for both TypeErrors below) and return what was returned in 3.9.0.

return cls.__create_ga(origin, args)

@classmethod
def __create_ga(cls, origin, args):
if not isinstance(args, tuple) or len(args) != 2:
raise TypeError(
"Callable must be used as Callable[[arg, ...], result].")
t_args, t_result = args
if isinstance(t_args, list):
ga_args = tuple(t_args) + (t_result,)
# This relaxes what t_args can be on purpose to allow things like
# PEP 612 ParamSpec. Responsibility for whether a user is using
# Callable[...] properly is deferred to static type checkers.
else:
ga_args = args
return super().__new__(cls, origin, ga_args)

def __repr__(self):
if len(self.__args__) == 2 and self.__args__[0] is Ellipsis:
return super().__repr__()
return (f'collections.abc.Callable'
f'[[{", ".join([_type_repr(a) for a in self.__args__[:-1]])}], '
f'{_type_repr(self.__args__[-1])}]')

def __reduce__(self):
args = self.__args__
if not (len(args) == 2 and args[0] is Ellipsis):
args = list(args[:-1]), args[-1]
return _CallableGenericAlias, (Callable, args)


def _type_repr(obj):
"""Return the repr() of an object, special-casing types (internal helper).

Copied from :mod:`typing` since collections.abc
shouldn't depend on that module.
"""
if isinstance(obj, GenericAlias):
return repr(obj)
if isinstance(obj, type):
if obj.__module__ == 'builtins':
return obj.__qualname__
return f'{obj.__module__}.{obj.__qualname__}'
if obj is Ellipsis:
return '...'
if isinstance(obj, FunctionType):
return obj.__name__
return repr(obj)


class Callable(metaclass=ABCMeta):

__slots__ = ()
Expand All@@ -423,7 +490,7 @@ def __subclasshook__(cls, C):
return _check_methods(C, "__call__")
return NotImplemented

__class_getitem__ = classmethod(GenericAlias)
__class_getitem__ = classmethod(_CallableGenericAlias)


### SETS ###
Expand Down
1 change: 1 addition & 0 deletions Lib/collections/abc.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
from _collections_abc import *
from _collections_abc import __all__
from _collections_abc import _CallableGenericAlias
58 changes: 57 additions & 1 deletion Lib/test/test_genericalias.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -62,7 +62,6 @@ class BaseTest(unittest.TestCase):
Iterable, Iterator,
Reversible,
Container, Collection,
Callable,
Mailbox, _PartialFile,
ContextVar, Token,
Field,
Expand DownExpand Up@@ -307,6 +306,63 @@ def test_no_kwargs(self):
with self.assertRaises(TypeError):
GenericAlias(bad=float)

def test_subclassing_types_genericalias(self):
class SubClass(GenericAlias): ...
alias = SubClass(list, int)
class Bad(GenericAlias):
def __new__(cls, *args, **kwargs):
super().__new__(cls, *args, **kwargs)

self.assertEqual(alias, list[int])
with self.assertRaises(TypeError):
Bad(list, int, bad=int)

def test_abc_callable(self):
# A separate test is needed for Callable since it uses a subclass of
# GenericAlias.
alias = Callable[[int, str], float]
with self.subTest("Testing subscription"):
self.assertIs(alias.__origin__, Callable)
self.assertEqual(alias.__args__, (int, str, float))
self.assertEqual(alias.__parameters__, ())

with self.subTest("Testing instance checks"):
self.assertIsInstance(alias, GenericAlias)

with self.subTest("Testing weakref"):
self.assertEqual(ref(alias)(), alias)

with self.subTest("Testing pickling"):
s = pickle.dumps(alias)
loaded = pickle.loads(s)
self.assertEqual(alias.__origin__, loaded.__origin__)
self.assertEqual(alias.__args__, loaded.__args__)
self.assertEqual(alias.__parameters__, loaded.__parameters__)

with self.subTest("Testing TypeVar substitution"):
C1 = Callable[[int, T], T]
C2 = Callable[[K, T], V]
C3 = Callable[..., T]
self.assertEqual(C1[str], Callable[[int, str], str])
self.assertEqual(C2[int, float, str], Callable[[int, float], str])
self.assertEqual(C3[int], Callable[..., int])

with self.subTest("Testing type erasure"):
class C1(Callable):
def __call__(self):
return None
a = C1[[int], T]
self.assertIs(a().__class__, C1)
self.assertEqual(a().__orig_class__, C1[[int], T])

# bpo-42195
with self.subTest("Testing collections.abc.Callable's consistency "
"with typing.Callable"):
c1 = typing.Callable[[int, str], dict]
c2 = Callable[[int, str], dict]
self.assertEqual(c1.__args__, c2.__args__)
self.assertEqual(hash(c1.__args__), hash(c2.__args__))


if __name__ == "__main__":
unittest.main()
12 changes: 7 additions & 5 deletions Lib/test/test_types.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -717,14 +717,16 @@ def test_or_type_operator_with_genericalias(self):
a = list[int]
b = list[str]
c = dict[float, str]
class SubClass(types.GenericAlias): ...
d = SubClass(list, float)
# equivalence with typing.Union
self.assertEqual(a | b | c, typing.Union[a, b, c])
self.assertEqual(a | b | c | d, typing.Union[a, b, c, d])
# de-duplicate
self.assertEqual(a | c | b | b | a | c, a | b | c)
self.assertEqual(a | c | b | b | a | c | d | d, a | b | c | d)
# order shouldn't matter
self.assertEqual(a | b, b | a)
self.assertEqual(repr(a | b | c),
"list[int] | list[str] | dict[float, str]")
self.assertEqual(a | b | d, b | a | d)
self.assertEqual(repr(a | b | c | d),
"list[int] | list[str] | dict[float, str] | list[float]")

class BadType(type):
def __eq__(self, other):
Expand Down
26 changes: 6 additions & 20 deletions Lib/test/test_typing.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -446,14 +446,6 @@ def test_cannot_instantiate(self):
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]
with self.assertRaises(TypeError):
Callable[int]

Expand DownExpand Up@@ -1807,10 +1799,9 @@ def barfoo2(x: CT): ...
def test_extended_generic_rules_subclassing(self):
class T1(Tuple[T, KT]): ...
class T2(Tuple[T, ...]): ...
class C1(Callable[[T], T]): ...
class C2(Callable[..., int]):
def __call__(self):
return None
class C1(typing.Container[T]):
def __contains__(self, item):
return False
Copy link
Member

@gvanrossumgvanrossumDec 5, 2020

Choose a reason for hiding this comment

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

So in the 3.9 backport this file should be untouched.


self.assertEqual(T1.__parameters__, (T, KT))
self.assertEqual(T1[int, str].__args__, (int, str))
Expand All@@ -1824,10 +1815,9 @@ def __call__(self):
## T2[int, str]

self.assertEqual(repr(C1[int]).split('.')[-1], 'C1[int]')
self.assertEqual(C2.__parameters__, ())
self.assertIsInstance(C2(), collections.abc.Callable)
self.assertIsSubclass(C2, collections.abc.Callable)
self.assertIsSubclass(C1, collections.abc.Callable)
self.assertEqual(C1.__parameters__, (T,))
self.assertIsInstance(C1(), collections.abc.Container)
self.assertIsSubclass(C1, collections.abc.Container)
self.assertIsInstance(T1(), tuple)
self.assertIsSubclass(T2, tuple)
with self.assertRaises(TypeError):
Expand DownExpand Up@@ -1861,10 +1851,6 @@ def test_type_erasure_special(self):
class MyTup(Tuple[T, T]): ...
self.assertIs(MyTup[int]().__class__, MyTup)
self.assertEqual(MyTup[int]().__orig_class__, MyTup[int])
class MyCall(Callable[..., T]):
def __call__(self): return None
self.assertIs(MyCall[T]().__class__, MyCall)
self.assertEqual(MyCall[T]().__orig_class__, MyCall[T])
class MyDict(typing.Dict[T, T]): ...
self.assertIs(MyDict[int]().__class__, MyDict)
self.assertEqual(MyDict[int]().__orig_class__, MyDict[int])
Expand Down
32 changes: 20 additions & 12 deletions Lib/typing.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -120,6 +120,16 @@
# namespace, but excluded from __all__ because they might stomp on
# legitimate imports of those modules.


def _type_convert(arg):
"""For converting None to type(None), and strings to ForwardRef."""
if arg is None:
return type(None)
if isinstance(arg, str):
return ForwardRef(arg)
return arg


def _type_check(arg, msg, is_argument=True):
"""Check that the argument is a type, and return it (internal helper).

Expand All@@ -136,10 +146,7 @@ def _type_check(arg, msg, is_argument=True):
if is_argument:
invalid_generic_forms = invalid_generic_forms + (ClassVar, Final)

if arg is None:
return type(None)
if isinstance(arg, str):
return ForwardRef(arg)
arg = _type_convert(arg)
if (isinstance(arg, _GenericAlias) and
arg.__origin__ in invalid_generic_forms):
raise TypeError(f"{arg} is not valid as type argument")
Expand DownExpand Up@@ -900,13 +907,13 @@ def __getitem__(self, params):
raise TypeError("Callable must be used as "
"Callable[[arg, ...], result].")
args, result = params
if args is Ellipsis:
params = (Ellipsis, result)
else:
if not isinstance(args, list):
raise TypeError(f"Callable[args, result]: args must be a list."
f" Got{args}")
# This relaxes what args can be on purpose to allow things like
# PEP 612 ParamSpec. Responsibility for whether a user is using
# Callable[...] properly is deferred to static type checkers.
if isinstance(args, list):
params = (tuple(args), result)
else:
params = (args, result)
return self.__getitem_inner__(params)

@_tp_cache
Expand All@@ -916,8 +923,9 @@ def __getitem_inner__(self, params):
result = _type_check(result, msg)
if args is Ellipsis:
return self.copy_with((_TypingEllipsis, result))
msg = "Callable[[arg, ...], result]: each arg must be a type."
args = tuple(_type_check(arg, msg) for arg in args)
if not isinstance(args, tuple):
args = (args,)
args = tuple(_type_convert(arg) for arg in args)
params = args + (result,)
return self.copy_with(params)

Expand Down
11 changes: 11 additions & 0 deletions Misc/NEWS.d/next/Core and Builtins/2020-11-20-00-57-47.bpo-42195.HeqcpS.rst
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
The ``__args__`` of the parameterized generics for :data:`typing.Callable`
and :class:`collections.abc.Callable` are now consistent. The ``__args__``
for :class:`collections.abc.Callable` are now flattened while
:data:`typing.Callable`'s have not changed. To allow this change,
:class:`types.GenericAlias` can now be subclassed and
``collections.abc.Callable``'s ``__class_getitem__`` will now return a subclass
of ``types.GenericAlias``. Tests for typing were also updated to not subclass
things like ``Callable[..., T]`` as that is not a valid base class. Finally,
both ``Callable``s no longer validate their ``argtypes``, in
``Callable[[argtypes], resulttype]`` to prepare for :pep:`612`. Patch by Ken Jin.

Loading