Skip to content

[3.9] bpo-42195: Ensure consistency of Callable's __args__ in collections.abc and typing (GH-23060) #23765

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 2 commits into from
Dec 14, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
76 changes: 75 additions & 1 deletion Lib/_collections_abc.py
Original file line number Diff line number Diff 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 Down Expand Up @@ -409,6 +413,76 @@ 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):
try:
return cls.__create_ga(origin, args)
except TypeError as exc:
import warnings
warnings.warn(f'{str(exc)} '
f'(This will raise a TypeError in Python 3.10.)',
DeprecationWarning)
return GenericAlias(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 +497,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
Original file line number Diff line number Diff 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
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ class BaseTest(unittest.TestCase):
Iterable, Iterator,
Reversible,
Container, Collection,
Callable,
Mailbox, _PartialFile,
ContextVar, Token,
Field,
Expand Down Expand 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()
8 changes: 0 additions & 8 deletions Lib/test/test_typing.py
Original file line number Diff line number Diff line change
Expand Up @@ -450,14 +450,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 Down
31 changes: 19 additions & 12 deletions Lib/typing.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,15 @@
# 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 @@ -134,10 +143,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 Down Expand Up @@ -859,13 +865,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 @@ -875,8 +881,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
Original file line number Diff line number Diff 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 types no longer validate their ``argtypes``, in
``Callable[[argtypes], resulttype]`` to prepare for :pep:`612`. Patch by Ken Jin.

60 changes: 39 additions & 21 deletions Objects/genericaliasobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -428,8 +428,8 @@ ga_getattro(PyObject *self, PyObject *name)
static PyObject *
ga_richcompare(PyObject *a, PyObject *b, int op)
{
if (!Py_IS_TYPE(a, &Py_GenericAliasType) ||
!Py_IS_TYPE(b, &Py_GenericAliasType) ||
if (!PyObject_TypeCheck(a, &Py_GenericAliasType) ||
!PyObject_TypeCheck(b, &Py_GenericAliasType) ||
(op != Py_EQ && op != Py_NE))
{
Py_RETURN_NOTIMPLEMENTED;
Expand Down Expand Up @@ -563,6 +563,29 @@ static PyGetSetDef ga_properties[] = {
{0}
};

/* A helper function to create GenericAlias' args tuple and set its attributes.
* Returns 1 on success, 0 on failure.
*/
static inline int
setup_ga(gaobject *alias, PyObject *origin, PyObject *args) {
if (!PyTuple_Check(args)) {
args = PyTuple_Pack(1, args);
if (args == NULL) {
return 0;
}
}
else {
Py_INCREF(args);
}

Py_INCREF(origin);
alias->origin = origin;
alias->args = args;
alias->parameters = NULL;
alias->weakreflist = NULL;
return 1;
}

static PyObject *
ga_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
{
Expand All @@ -574,7 +597,15 @@ ga_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
}
PyObject *origin = PyTuple_GET_ITEM(args, 0);
PyObject *arguments = PyTuple_GET_ITEM(args, 1);
return Py_GenericAlias(origin, arguments);
gaobject *self = (gaobject *)type->tp_alloc(type, 0);
if (self == NULL) {
return NULL;
}
if (!setup_ga(self, origin, arguments)) {
type->tp_free((PyObject *)self);
return NULL;
}
return (PyObject *)self;
}

// TODO:
Expand All @@ -594,7 +625,7 @@ PyTypeObject Py_GenericAliasType = {
.tp_hash = ga_hash,
.tp_call = ga_call,
.tp_getattro = ga_getattro,
.tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC,
.tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_BASETYPE,
.tp_traverse = ga_traverse,
.tp_richcompare = ga_richcompare,
.tp_weaklistoffset = offsetof(gaobject, weakreflist),
Expand All @@ -609,27 +640,14 @@ PyTypeObject Py_GenericAliasType = {
PyObject *
Py_GenericAlias(PyObject *origin, PyObject *args)
{
if (!PyTuple_Check(args)) {
args = PyTuple_Pack(1, args);
if (args == NULL) {
return NULL;
}
}
else {
Py_INCREF(args);
}

gaobject *alias = PyObject_GC_New(gaobject, &Py_GenericAliasType);
if (alias == NULL) {
Py_DECREF(args);
return NULL;
}

Py_INCREF(origin);
alias->origin = origin;
alias->args = args;
alias->parameters = NULL;
alias->weakreflist = NULL;
if (!setup_ga(alias, origin, args)) {
PyObject_GC_Del((PyObject *)alias);
return NULL;
}
_PyObject_GC_TRACK(alias);
return (PyObject *)alias;
}