bpo-32492: Tweak _collections._tuplegetter. (GH-11367)

* Replace the docstrings cache with sys.intern().
* Improve tests.
* Unify names of tp_descr_get and tp_descr_set functions.

Author: serhiy-storchaka

Lib/collections/__init__.py

@@ -316,8 +316,6 @@ def __eq__(self, other):
 except ImportError:
     _tuplegetter = lambda index, doc: property(_itemgetter(index), doc=doc)
 
-_nt_itemgetters = {}
-
 def namedtuple(typename, field_names, *, rename=False, defaults=None, module=None):
     """Returns a new subclass of tuple with named fields.
 
@@ -456,16 +454,9 @@ def __getnewargs__(self):
         '_asdict': _asdict,
         '__getnewargs__': __getnewargs__,
     }
-    cache = _nt_itemgetters
     for index, name in enumerate(field_names):
-        try:
-            doc = cache[index]
-        except KeyError:
-            doc = f'Alias for field number {index}'
-            cache[index] = doc
-
-        tuplegetter_object = _tuplegetter(index, doc)
-        class_namespace[name] = tuplegetter_object
+        doc = _sys.intern(f'Alias for field number {index}')
+        class_namespace[name] = _tuplegetter(index, doc)
 
     result = type(typename, (tuple,), class_namespace)
 

Lib/test/test_collections.py

@@ -3,6 +3,7 @@
 import collections
 import copy
 import doctest
+import inspect
 import operator
 import pickle
 from random import choice, randrange
@@ -281,20 +282,50 @@ def test_defaults(self):
         self.assertEqual(Point(1), (1, 20))
         self.assertEqual(Point(), (10, 20))
 
+    def test_readonly(self):
+        Point = namedtuple('Point', 'x y')
+        p = Point(11, 22)
+        with self.assertRaises(AttributeError):
+            p.x = 33
+        with self.assertRaises(AttributeError):
+            del p.x
+        with self.assertRaises(TypeError):
+            p[0] = 33
+        with self.assertRaises(TypeError):
+            del p[0]
+        self.assertEqual(p.x, 11)
+        self.assertEqual(p[0], 11)
 
     @unittest.skipIf(sys.flags.optimize >= 2,
                      "Docstrings are omitted with -O2 and above")
     def test_factory_doc_attr(self):
         Point = namedtuple('Point', 'x y')
         self.assertEqual(Point.__doc__, 'Point(x, y)')
+        Point.__doc__ = '2D point'
+        self.assertEqual(Point.__doc__, '2D point')
 
     @unittest.skipIf(sys.flags.optimize >= 2,
                      "Docstrings are omitted with -O2 and above")
-    def test_doc_writable(self):
+    def test_field_doc(self):
         Point = namedtuple('Point', 'x y')
         self.assertEqual(Point.x.__doc__, 'Alias for field number 0')
+        self.assertEqual(Point.y.__doc__, 'Alias for field number 1')
         Point.x.__doc__ = 'docstring for Point.x'
         self.assertEqual(Point.x.__doc__, 'docstring for Point.x')
+        # namedtuple can mutate doc of descriptors independently
+        Vector = namedtuple('Vector', 'x y')
+        self.assertEqual(Vector.x.__doc__, 'Alias for field number 0')
+        Vector.x.__doc__ = 'docstring for Vector.x'
+        self.assertEqual(Vector.x.__doc__, 'docstring for Vector.x')
+
+    @support.cpython_only
+    @unittest.skipIf(sys.flags.optimize >= 2,
+                     "Docstrings are omitted with -O2 and above")
+    def test_field_doc_reuse(self):
+        P = namedtuple('P', ['m', 'n'])
+        Q = namedtuple('Q', ['o', 'p'])
+        self.assertIs(P.m.__doc__, Q.o.__doc__)
+        self.assertIs(P.n.__doc__, Q.p.__doc__)
 
     def test_name_fixer(self):
         for spec, renamed in [
@@ -319,16 +350,18 @@ def test_instance(self):
         self.assertEqual(p, Point(y=22, x=11))
         self.assertEqual(p, Point(*(11, 22)))
         self.assertEqual(p, Point(**dict(x=11, y=22)))
-        self.assertRaises(TypeError, Point, 1)                              # too few args
-        self.assertRaises(TypeError, Point, 1, 2, 3)                        # too many args
-        self.assertRaises(TypeError, eval, 'Point(XXX=1, y=2)', locals())   # wrong keyword argument
-        self.assertRaises(TypeError, eval, 'Point(x=1)', locals())          # missing keyword argument
+        self.assertRaises(TypeError, Point, 1)          # too few args
+        self.assertRaises(TypeError, Point, 1, 2, 3)    # too many args
+        with self.assertRaises(TypeError):              # wrong keyword argument
+            Point(XXX=1, y=2)
+        with self.assertRaises(TypeError):              # missing keyword argument
+            Point(x=1)
         self.assertEqual(repr(p), 'Point(x=11, y=22)')
         self.assertNotIn('__weakref__', dir(p))
-        self.assertEqual(p, Point._make([11, 22]))                          # test _make classmethod
-        self.assertEqual(p._fields, ('x', 'y'))                             # test _fields attribute
-        self.assertEqual(p._replace(x=1), (1, 22))                          # test _replace method
-        self.assertEqual(p._asdict(), dict(x=11, y=22))                     # test _asdict method
+        self.assertEqual(p, Point._make([11, 22]))      # test _make classmethod
+        self.assertEqual(p._fields, ('x', 'y'))         # test _fields attribute
+        self.assertEqual(p._replace(x=1), (1, 22))      # test _replace method
+        self.assertEqual(p._asdict(), dict(x=11, y=22)) # test _asdict method
 
         try:
             p._replace(x=1, error=2)
@@ -360,11 +393,15 @@ def test_tupleness(self):
         x, y = p
         self.assertEqual(p, (x, y))                                         # unpacks like a tuple
         self.assertEqual((p[0], p[1]), (11, 22))                            # indexable like a tuple
-        self.assertRaises(IndexError, p.__getitem__, 3)
+        with self.assertRaises(IndexError):
+            p[3]
+        self.assertEqual(p[-1], 22)
+        self.assertEqual(hash(p), hash((11, 22)))
 
         self.assertEqual(p.x, x)
         self.assertEqual(p.y, y)
-        self.assertRaises(AttributeError, eval, 'p.z', locals())
+        with self.assertRaises(AttributeError):
+            p.z
 
     def test_odd_sizes(self):
         Zero = namedtuple('Zero', '')
@@ -514,13 +551,13 @@ class Point(namedtuple('_Point', ['x', 'y'])):
         a.w = 5
         self.assertEqual(a.__dict__, {'w': 5})
 
-    def test_namedtuple_can_mutate_doc_of_descriptors_independently(self):
-        A = namedtuple('A', 'x y')
-        B = namedtuple('B', 'x y')
-        A.x.__doc__ = 'foo'
-        B.x.__doc__ = 'bar'
-        self.assertEqual(A.x.__doc__, 'foo')
-        self.assertEqual(B.x.__doc__, 'bar')
+    def test_field_descriptor(self):
+        Point = namedtuple('Point', 'x y')
+        p = Point(11, 22)
+        self.assertTrue(inspect.isdatadescriptor(Point.x))
+        self.assertEqual(Point.x.__get__(p), 11)
+        self.assertRaises(AttributeError, Point.x.__set__, p, 33)
+        self.assertRaises(AttributeError, Point.x.__delete__, p)
 
 
 ################################################################################

Lib/test/test_pydoc.py

@@ -687,6 +687,16 @@ def test_help_output_redirect(self):
         finally:
             pydoc.getpager = getpager_old
 
+    def test_namedtuple_fields(self):
+        Person = namedtuple('Person', ['nickname', 'firstname'])
+        with captured_stdout() as help_io:
+            pydoc.help(Person)
+        helptext = help_io.getvalue()
+        self.assertIn("nickname", helptext)
+        self.assertIn("firstname", helptext)
+        self.assertIn("Alias for field number 0", helptext)
+        self.assertIn("Alias for field number 1", helptext)
+
     def test_namedtuple_public_underscore(self):
         NT = namedtuple('NT', ['abc', 'def'], rename=True)
         with captured_stdout() as help_io:

Modules/_collectionsmodule.c

@@ -2336,7 +2336,7 @@ _count_elements(PyObject *self, PyObject *args)
     Py_RETURN_NONE;
 }
 
-/* Helper functions for namedtuples */
+/* Helper function for namedtuple() ************************************/
 
 typedef struct {
     PyObject_HEAD
@@ -2369,9 +2369,11 @@ tuplegetter_new_impl(PyTypeObject *type, Py_ssize_t index, PyObject *doc)
 }
 
 static PyObject *
-tuplegetterdescr_get(PyObject *self, PyObject *obj, PyObject *type)
+tuplegetter_descr_get(PyObject *self, PyObject *obj, PyObject *type)
 {
+    Py_ssize_t index = ((_tuplegetterobject*)self)->index;
     PyObject *result;
+
     if (obj == NULL) {
         Py_INCREF(self);
         return self;
@@ -2384,13 +2386,11 @@ tuplegetterdescr_get(PyObject *self, PyObject *obj, PyObject *type)
         PyErr_Format(PyExc_TypeError,
                      "descriptor for index '%d' for tuple subclasses "
                      "doesn't apply to '%s' object",
-                     ((_tuplegetterobject*)self)->index,
+                     index,
                      obj->ob_type->tp_name);
         return NULL;
     }
 
-    Py_ssize_t index = ((_tuplegetterobject*)self)->index;
-
     if (!valid_index(index, PyTuple_GET_SIZE(obj))) {
         PyErr_SetString(PyExc_IndexError, "tuple index out of range");
         return NULL;
@@ -2402,7 +2402,7 @@ tuplegetterdescr_get(PyObject *self, PyObject *obj, PyObject *type)
 }
 
 static int
-tuplegetter_set(PyObject *self, PyObject *obj, PyObject *value)
+tuplegetter_descr_set(PyObject *self, PyObject *obj, PyObject *value)
 {
     if (value == NULL) {
         PyErr_SetString(PyExc_AttributeError, "can't delete attribute");
@@ -2476,8 +2476,8 @@ static PyTypeObject tuplegetter_type = {
     0,                                          /* tp_getset */
     0,                                          /* tp_base */
     0,                                          /* tp_dict */
-    tuplegetterdescr_get,                       /* tp_descr_get */
-    tuplegetter_set,                            /* tp_descr_set */
+    tuplegetter_descr_get,                      /* tp_descr_get */
+    tuplegetter_descr_set,                      /* tp_descr_set */
     0,                                          /* tp_dictoffset */
     0,                                          /* tp_init */
     0,                                          /* tp_alloc */