Address most of Guido's reviews (tests failing on purpose)

python · gvanrossum · Dec 13, 2020 · Oct 31, 2020 · Oct 31, 2020 · Oct 31, 2020
commit 3ddca0664b4e53d525ace1947128ed11ff6d452c
diff --git a/Lib/_collections_abc.py b/Lib/_collections_abc.py
@@ -24,11 +24,6 @@ def _f(): pass
            "MappingView", "KeysView", "ItemsView", "ValuesView",
            "Sequence", "MutableSequence",
            "ByteString",
-           # The following classse are to allow pickling, not actually
-           # meant to be imported.
-           "_CallableGenericAlias",
-           "_PosArgs",
-           "_PosArgsGenericAlias"
            ]
 
 # This module has been renamed from collections.abc to _collections_abc to
@@ -422,17 +417,19 @@ class _PosArgsGenericAlias(GenericAlias):
     """ Internal class specifically to represent positional arguments in
     ``_CallableGenericAlias``.
     """
+    __slots__ = ()
+
     def __repr__(self):
         return f"{__name__}._PosArgsGenericAlias" \
                f"[{', '.join(_type_repr(t) for t in self.__args__)}]"
 
     def __eq__(self, other):
         o_cls = other.__class__
-        if not (o_cls.__module__ == "typing" and o_cls.__name__
-                == "_GenericAlias" or isinstance(other, GenericAlias)):
-            return NotImplemented
-        return (self.__origin__ == other.__origin__
-                and self.__args__ == other.__args__)
+        if ((o_cls.__module__ == "typing" and o_cls.__name__
+                == "_GenericAlias") or isinstance(other, GenericAlias)):
+            return (self.__origin__ == other.__origin__
+                    and self.__args__ == other.__args__)
+        return NotImplemented
 
     def __hash__(self):
         return hash((self.__origin__, self.__args__))
@@ -445,39 +442,37 @@ class _PosArgs:
     def __class_getitem__(cls, item):
         return _PosArgsGenericAlias(tuple, item)
 
-# _PosArgs = type("_PosArgs", (tuple, ), {})
 
 class _CallableGenericAlias(GenericAlias):
     """ Internal class specifically for consistency between the ``__args__`` of
     ``collections.abc.Callable``'s and ``typing.Callable``'s ``GenericAlias``.
     """
 
-    def __new__(cls, *args, **kwargs):
-        if not isinstance(args, tuple) or len(args) != 2:
-            raise TypeError("Callable must be used as Callable[[arg, ...], result]")
-        origin, _args = args
+    def __new__(cls, origin, _args, **kwargs):
         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 not isinstance(t_args, (list, EllipsisType)):
-            raise TypeError("Callable[args, result]: args must be a list "
-                            f"or Ellipsis. Got {_type_repr(t_args)}")
+            raise TypeError(f"Callable[args, result]: args must be a list or Ellipsis. "
+                            f"Got {_type_repr(t_args)}")
 
-        ga_args = (_args if t_args is Ellipsis
-                   else (_PosArgs[tuple(t_args)], t_result))
+        if t_args is Ellipsis:
+            ga_args = _args
+        else:
+            ga_args = _PosArgs[tuple(t_args)], t_result
 
         return super().__new__(cls, origin, ga_args)
 
 
     def __repr__(self):
+        if len(self.__args__) == 2 and self.__args__[0] is Ellipsis:
+            return super().__repr__()
         t_args = self.__args__[0]
+        if t_args.__args__ == ((),):
+            t_args_repr = '[]'
+        else:
+            t_args_repr = f'[{", ".join(_type_repr(a) for a in t_args.__args__)}]'
         origin = _type_repr(self.__origin__)
-
-        if len(self.__args__) == 2 and t_args is Ellipsis:
-            return super().__repr__()
-        t_args_repr = ('[]' if t_args.__args__ == ((),) else
-                       f'[{", ".join(_type_repr(a) for a in t_args.__args__)}]')
-
         return f"{origin}[{t_args_repr}, {_type_repr(self.__args__[-1])}]"
 
     def __reduce__(self):
@@ -490,7 +485,7 @@ def __reduce__(self):
 def _type_repr(obj):
     """Return the repr() of an object, special-casing types (internal helper).
 
-    Borrowed from :mod:`typing` without importing since collections.abc
+    Copied from :mod:`typing` since collections.abc
     shouldn't depend on that module.
     """
     if isinstance(obj, GenericAlias):

diff --git a/Lib/collections/abc.py b/Lib/collections/abc.py
@@ -1,2 +1,3 @@
 from _collections_abc import *
 from _collections_abc import __all__
+from _collections_abc import _CallableGenericAlias, _PosArgs, _PosArgsGenericAlias
diff --git a/Lib/test/test_genericalias.py b/Lib/test/test_genericalias.py
@@ -6,6 +6,7 @@
     defaultdict, deque, OrderedDict, Counter, UserDict, UserList
 )
 from collections.abc import *
+from collections.abc import _PosArgs
 from concurrent.futures import Future
 from concurrent.futures.thread import _WorkItem
 from contextlib import AbstractContextManager, AbstractAsyncContextManager
@@ -308,7 +309,7 @@ def test_abc_callable(self):
             self.assertEqual(alias.__args__, (_PosArgs[int, str], float))
             self.assertEqual(alias.__parameters__, ())
 
-        with self.subTest("Testing nstance checks"):
+        with self.subTest("Testing instance checks"):
             self.assertIsInstance(alias, GenericAlias)
 
         invalid_params = ('Callable[int]', 'Callable[int, str]')

diff --git a/Lib/test/test_typing.py b/Lib/test/test_typing.py
@@ -1818,7 +1818,7 @@ def __call__(self):
         ## with self.assertRaises(TypeError):
         ##     T2[int, str]
 
-        self.assertEqual(repr(C1[[int], int]).split('.')[-1], 'C1[[int], int]')
+        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)
@@ -1858,8 +1858,8 @@ class MyTup(Tuple[T, T]): ...
         self.assertEqual(MyTup[int]().__orig_class__, MyTup[int])
         class MyCall(Callable[..., T]):
             def __call__(self): return None
-        self.assertIs(MyCall[[T], T]().__class__, MyCall)
-        self.assertEqual(MyCall[[T], T]().__orig_class__, MyCall[[T], T])
+        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])

diff --git a/Lib/typing.py b/Lib/typing.py
@@ -114,7 +114,6 @@
     'Text',
     'TYPE_CHECKING',
     'TypeAlias',
-    '_PosArgs',  # Not meant to be imported, just for pickling.
 ]
 
 # The pseudo-submodules 're' and 'io' are part of the public
@@ -878,11 +877,13 @@ def __ror__(self, right):
 class _CallableGenericAlias(_GenericAlias, _root=True):
     def __repr__(self):
         assert self._name == 'Callable'
-        t_args = self.__args__[0]
-        if len(self.__args__) == 2 and t_args is Ellipsis:
+        if len(self.__args__) == 2 and self.__args__[0] is Ellipsis:
             return super().__repr__()
-        t_args_repr = ('[]' if t_args.__args__ == ((),) else
-                       f'[{", ".join(_type_repr(a) for a in t_args.__args__)}]')
+        t_args = self.__args__[0]
+        if t_args.__args__ == ((),):
+            t_args_repr = '[]'
+        else:
+            t_args_repr = f'[{", ".join(_type_repr(a) for a in t_args.__args__)}]'
         return (f'typing.Callable'
                 f'[{t_args_repr}, '
                 f'{_type_repr(self.__args__[-1])}]')

diff --git a/Objects/genericaliasobject.c b/Objects/genericaliasobject.c
@@ -614,17 +614,14 @@ ga_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
     if (!_PyArg_CheckPositional("GenericAlias", PyTuple_GET_SIZE(args), 2, 2)) {
         return NULL;
     }
-
     PyObject *origin = PyTuple_GET_ITEM(args, 0);
     PyObject *arguments = PyTuple_GET_ITEM(args, 1);
-
     PyObject *self = (PyObject *)create_ga(type, origin, arguments);
     if (self == NULL) {
         Py_DECREF(origin);
         Py_DECREF(arguments);
         return NULL;
     }
-
     return self;
 }