https://github.com/python/cpython/commit/052b2dfdc967a8c061ff9561534e905009b... commit: 052b2dfdc967a8c061ff9561534e905009b88b8c branch: master author: Serhiy Storchaka <storchaka@gmail.com> committer: GitHub <noreply@github.com> date: 2018-12-31T14:15:16+02:00 summary: 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. files: M Lib/collections/__init__.py M Lib/test/test_collections.py M Lib/test/test_pydoc.py M Modules/_collectionsmodule.c diff --git a/Lib/collections/__init__.py b/Lib/collections/__init__.py index 0b74c3f8915f..c31d7b79185d 100644 --- a/Lib/collections/__init__.py +++ b/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) diff --git a/Lib/test/test_collections.py b/Lib/test/test_collections.py index c0815947f908..74372d28ed05 100644 --- a/Lib/test/test_collections.py +++ b/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) ################################################################################ diff --git a/Lib/test/test_pydoc.py b/Lib/test/test_pydoc.py index c58a8b13e76d..ffe80fc06fc6 100644 --- a/Lib/test/test_pydoc.py +++ b/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: diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index cc325e1aaac4..a2b683e16663 100644 --- a/Modules/_collectionsmodule.c +++ b/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 */