From b19523aa70aae30611e59f8461d25391620b9967 Mon Sep 17 00:00:00 2001 From: Sergei Izmailov Date: Mon, 9 Nov 2020 12:03:08 +0300 Subject: [PATCH 01/17] Handle `doc` argument of `property.__init__` in subclasses --- Lib/test/test_pydoc.py | 19 +++++++++++++++++++ Objects/descrobject.c | 39 ++++++++++++++++++++++----------------- 2 files changed, 41 insertions(+), 17 deletions(-) diff --git a/Lib/test/test_pydoc.py b/Lib/test/test_pydoc.py index 9c900c3e8ee0af..fddeae71c7f8fe 100644 --- a/Lib/test/test_pydoc.py +++ b/Lib/test/test_pydoc.py @@ -449,6 +449,25 @@ def test_issue8225(self): result, doc_loc = get_pydoc_text(xml.etree) self.assertEqual(doc_loc, "", "MODULE DOCS incorrectly includes a link") + def test_issue41287(self): + # Test issue41287 to ensure property subclass handles `doc` argument correctly + + class Property(property): + """A subclass of builtin property""" + + self.assertEqual(Property.__doc__, "A subclass of builtin property", "Docstring of `property` subclass is ignored") + + doc = Property(None, None, None, "issue 41287 is fixed").__doc__ + self.assertEqual(doc, "issue 41287 is fixed", "Subclasses of `property` ignores `doc` constructor argument") + + def getter(x): + """Getter docstring""" + doc = Property(getter, None, None, "issue 41287 is fixed").__doc__ + self.assertEqual(doc, "issue 41287 is fixed", "Getter overrides explicit property docstring docstring") + + doc = Property(getter, None, None, None).__doc__ + self.assertEqual(doc, "Getter docstring", "Getter docstring is not picked-up") + def test_getpager_with_stdin_none(self): previous_stdin = sys.stdin try: diff --git a/Objects/descrobject.c b/Objects/descrobject.c index 7cbfe8d9c19407..32866f723bbf33 100644 --- a/Objects/descrobject.c +++ b/Objects/descrobject.c @@ -1781,39 +1781,44 @@ property_init_impl(propertyobject *self, PyObject *fget, PyObject *fset, Py_XINCREF(fget); Py_XINCREF(fset); Py_XINCREF(fdel); - Py_XINCREF(doc); Py_XSETREF(self->prop_get, fget); Py_XSETREF(self->prop_set, fset); Py_XSETREF(self->prop_del, fdel); - Py_XSETREF(self->prop_doc, doc); + Py_XSETREF(self->prop_doc, NULL); Py_XSETREF(self->prop_name, NULL); self->getter_doc = 0; + PyObject *get_doc = NULL; /* if no docstring given and the getter has one, use that one */ if ((doc == NULL || doc == Py_None) && fget != NULL) { - PyObject *get_doc; int rc = _PyObject_LookupAttr(fget, &_Py_ID(__doc__), &get_doc); if (rc <= 0) { return rc; } - if (Py_IS_TYPE(self, &PyProperty_Type)) { - Py_XSETREF(self->prop_doc, get_doc); - } - else { - /* If this is a property subclass, put __doc__ - in dict of the subclass instance instead, - otherwise it gets shadowed by __doc__ in the - class's dict. */ - int err = PyObject_SetAttr( - (PyObject *)self, &_Py_ID(__doc__), get_doc); - Py_DECREF(get_doc); - if (err < 0) - return -1; - } self->getter_doc = 1; } + else { + get_doc = doc; + Py_XINCREF(get_doc); + } + + + if (Py_IS_TYPE(self, &PyProperty_Type)) { + Py_XSETREF(self->prop_doc, get_doc); + } + else { + /* If this is a property subclass, put __doc__ + in dict of the subclass instance instead, + otherwise it gets shadowed by __doc__ in the + class's dict. */ + int err = PyObject_SetAttr( + (PyObject *)self, &_Py_ID(__doc__), get_doc); + Py_XDECREF(get_doc); + if (err < 0) + return -1; + } return 0; } From 0745f700dbf906b77dd9cec2678d95c59c3088c4 Mon Sep 17 00:00:00 2001 From: Sergei Izmailov Date: Sun, 3 Apr 2022 11:26:06 +0300 Subject: [PATCH 02/17] Add NEWS entry --- .../NEWS.d/next/Library/2022-04-03-11-25-02.bpo-41287.8CTdwf.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Library/2022-04-03-11-25-02.bpo-41287.8CTdwf.rst diff --git a/Misc/NEWS.d/next/Library/2022-04-03-11-25-02.bpo-41287.8CTdwf.rst b/Misc/NEWS.d/next/Library/2022-04-03-11-25-02.bpo-41287.8CTdwf.rst new file mode 100644 index 00000000000000..aeb31c4cf85d5e --- /dev/null +++ b/Misc/NEWS.d/next/Library/2022-04-03-11-25-02.bpo-41287.8CTdwf.rst @@ -0,0 +1 @@ +Handle doc argument of property.__init__ in subclasses From ebec602f432fa4dd522de46d64f5a1b96ce98307 Mon Sep 17 00:00:00 2001 From: Sergei Izmailov Date: Tue, 5 Apr 2022 13:02:34 +0300 Subject: [PATCH 03/17] Update Misc/NEWS.d/next/Library/2022-04-03-11-25-02.bpo-41287.8CTdwf.rst Co-authored-by: Jelle Zijlstra --- .../next/Library/2022-04-03-11-25-02.bpo-41287.8CTdwf.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2022-04-03-11-25-02.bpo-41287.8CTdwf.rst b/Misc/NEWS.d/next/Library/2022-04-03-11-25-02.bpo-41287.8CTdwf.rst index aeb31c4cf85d5e..ef80ec664c4a86 100644 --- a/Misc/NEWS.d/next/Library/2022-04-03-11-25-02.bpo-41287.8CTdwf.rst +++ b/Misc/NEWS.d/next/Library/2022-04-03-11-25-02.bpo-41287.8CTdwf.rst @@ -1 +1 @@ -Handle doc argument of property.__init__ in subclasses +Fix handling of the ``doc`` argument in subclasses of :func:`property`. From f09e6acf396c4728cef810a1a675a9d60c0ad2fa Mon Sep 17 00:00:00 2001 From: Sergei Izmailov Date: Tue, 5 Apr 2022 20:30:18 +0300 Subject: [PATCH 04/17] Move test to test_property.py --- Lib/test/test_property.py | 20 ++++++++++++++++++++ Lib/test/test_pydoc.py | 19 ------------------- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/Lib/test/test_property.py b/Lib/test/test_property.py index d91ad1c191275e..73d66575bcd29e 100644 --- a/Lib/test/test_property.py +++ b/Lib/test/test_property.py @@ -237,6 +237,26 @@ def spam(self): else: raise Exception("AttributeError not raised") + @unittest.skipIf(sys.flags.optimize >= 2, + "Docstrings are omitted with -O2 and above") + def test_issue41287(self): + + self.assertEqual(PropertySub.__doc__, "This is a subclass of property", + "Docstring of `property` subclass is ignored") + + doc = PropertySub(None, None, None, "issue 41287 is fixed").__doc__ + self.assertEqual(doc, "issue 41287 is fixed", + "Subclasses of `property` ignores `doc` constructor argument") + + def getter(x): + """Getter docstring""" + doc = PropertySub(getter, None, None, "issue 41287 is fixed").__doc__ + self.assertEqual(doc, "issue 41287 is fixed", + "Getter overrides explicit property docstring docstring") + + doc = PropertySub(getter, None, None, None).__doc__ + self.assertEqual(doc, "Getter docstring", "Getter docstring is not picked-up") + @unittest.skipIf(sys.flags.optimize >= 2, "Docstrings are omitted with -O2 and above") def test_docstring_copy(self): diff --git a/Lib/test/test_pydoc.py b/Lib/test/test_pydoc.py index fddeae71c7f8fe..9c900c3e8ee0af 100644 --- a/Lib/test/test_pydoc.py +++ b/Lib/test/test_pydoc.py @@ -449,25 +449,6 @@ def test_issue8225(self): result, doc_loc = get_pydoc_text(xml.etree) self.assertEqual(doc_loc, "", "MODULE DOCS incorrectly includes a link") - def test_issue41287(self): - # Test issue41287 to ensure property subclass handles `doc` argument correctly - - class Property(property): - """A subclass of builtin property""" - - self.assertEqual(Property.__doc__, "A subclass of builtin property", "Docstring of `property` subclass is ignored") - - doc = Property(None, None, None, "issue 41287 is fixed").__doc__ - self.assertEqual(doc, "issue 41287 is fixed", "Subclasses of `property` ignores `doc` constructor argument") - - def getter(x): - """Getter docstring""" - doc = Property(getter, None, None, "issue 41287 is fixed").__doc__ - self.assertEqual(doc, "issue 41287 is fixed", "Getter overrides explicit property docstring docstring") - - doc = Property(getter, None, None, None).__doc__ - self.assertEqual(doc, "Getter docstring", "Getter docstring is not picked-up") - def test_getpager_with_stdin_none(self): previous_stdin = sys.stdin try: From dcab455a9055238174fd972933b3e977678a148d Mon Sep 17 00:00:00 2001 From: Sergei Izmailov Date: Tue, 5 Apr 2022 20:35:15 +0300 Subject: [PATCH 05/17] Rename local get_doc -> prop_doc --- Objects/descrobject.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Objects/descrobject.c b/Objects/descrobject.c index 32866f723bbf33..f4a4635dd0dbf8 100644 --- a/Objects/descrobject.c +++ b/Objects/descrobject.c @@ -1789,24 +1789,24 @@ property_init_impl(propertyobject *self, PyObject *fget, PyObject *fset, Py_XSETREF(self->prop_name, NULL); self->getter_doc = 0; - PyObject *get_doc = NULL; + PyObject *prop_doc = NULL; /* if no docstring given and the getter has one, use that one */ if ((doc == NULL || doc == Py_None) && fget != NULL) { - int rc = _PyObject_LookupAttr(fget, &_Py_ID(__doc__), &get_doc); + int rc = _PyObject_LookupAttr(fget, &_Py_ID(__doc__), &prop_doc); if (rc <= 0) { return rc; } self->getter_doc = 1; } else { - get_doc = doc; - Py_XINCREF(get_doc); + prop_doc = doc; + Py_XINCREF(prop_doc); } if (Py_IS_TYPE(self, &PyProperty_Type)) { - Py_XSETREF(self->prop_doc, get_doc); + Py_XSETREF(self->prop_doc, prop_doc); } else { /* If this is a property subclass, put __doc__ @@ -1814,8 +1814,8 @@ property_init_impl(propertyobject *self, PyObject *fget, PyObject *fset, otherwise it gets shadowed by __doc__ in the class's dict. */ int err = PyObject_SetAttr( - (PyObject *)self, &_Py_ID(__doc__), get_doc); - Py_XDECREF(get_doc); + (PyObject *)self, &_Py_ID(__doc__), prop_doc); + Py_XDECREF(prop_doc); if (err < 0) return -1; } From de2c3c0a29a119c68da8a53c538db6add0cf7656 Mon Sep 17 00:00:00 2001 From: Sergei Izmailov Date: Tue, 5 Apr 2022 20:37:27 +0300 Subject: [PATCH 06/17] Add prop_doc != NULL check --- Objects/descrobject.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Objects/descrobject.c b/Objects/descrobject.c index f4a4635dd0dbf8..92baf4eed52129 100644 --- a/Objects/descrobject.c +++ b/Objects/descrobject.c @@ -1808,7 +1808,7 @@ property_init_impl(propertyobject *self, PyObject *fget, PyObject *fset, if (Py_IS_TYPE(self, &PyProperty_Type)) { Py_XSETREF(self->prop_doc, prop_doc); } - else { + else if (prop_doc != NULL) { /* If this is a property subclass, put __doc__ in dict of the subclass instance instead, otherwise it gets shadowed by __doc__ in the From bd4cdca9ec312bae3aa053ac3d3f7ce0bcc070f0 Mon Sep 17 00:00:00 2001 From: Sergei Izmailov Date: Tue, 5 Apr 2022 20:57:37 +0300 Subject: [PATCH 07/17] Add more tests --- Lib/test/test_property.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/Lib/test/test_property.py b/Lib/test/test_property.py index 73d66575bcd29e..882950b6810292 100644 --- a/Lib/test/test_property.py +++ b/Lib/test/test_property.py @@ -219,6 +219,9 @@ def test_property_set_name_incorrect_args(self): class PropertySub(property): """This is a subclass of property""" +class PropertySubWoDoc(property): + pass + class PropertySubSlots(property): """This is a subclass of property that defines __slots__""" __slots__ = () @@ -257,6 +260,27 @@ def getter(x): doc = PropertySub(getter, None, None, None).__doc__ self.assertEqual(doc, "Getter docstring", "Getter docstring is not picked-up") + doc = PropertySubWoDoc(getter, None, None, "issue 41287 is fixed").__doc__ + self.assertEqual(doc, "issue 41287 is fixed", + "Getter overrides explicit property docstring docstring") + + doc = PropertySubWoDoc(getter, None, None, None).__doc__ + self.assertEqual(doc, "Getter docstring", "Getter docstring is not picked-up") + + def getter_wo_doc(x): + pass + doc = PropertySub(getter_wo_doc, None, None, "issue 41287 is fixed").__doc__ + self.assertEqual(doc, "issue 41287 is fixed", + "Getter overrides explicit property docstring docstring") + + doc = PropertySub(getter_wo_doc, None, None, None).__doc__ + self.assertEqual(doc, "This is a subclass of property", + "Getter without docstring overrides PropertySub.__doc__") + + doc = PropertySubWoDoc(getter_wo_doc, None, None, "issue 41287 is fixed").__doc__ + self.assertEqual(doc, "issue 41287 is fixed", + "Getter overrides explicit property docstring docstring") + @unittest.skipIf(sys.flags.optimize >= 2, "Docstrings are omitted with -O2 and above") def test_docstring_copy(self): From 1a1247e3aaf7e9c22c5c6b5b8e0d192970131a8b Mon Sep 17 00:00:00 2001 From: Sergei Izmailov Date: Tue, 5 Apr 2022 23:32:56 +0300 Subject: [PATCH 08/17] Fix test --- Lib/test/test_property.py | 35 +++++++++++++---------------------- 1 file changed, 13 insertions(+), 22 deletions(-) diff --git a/Lib/test/test_property.py b/Lib/test/test_property.py index 882950b6810292..532b2cdb275054 100644 --- a/Lib/test/test_property.py +++ b/Lib/test/test_property.py @@ -253,33 +253,24 @@ def test_issue41287(self): def getter(x): """Getter docstring""" - doc = PropertySub(getter, None, None, "issue 41287 is fixed").__doc__ - self.assertEqual(doc, "issue 41287 is fixed", - "Getter overrides explicit property docstring docstring") - - doc = PropertySub(getter, None, None, None).__doc__ - self.assertEqual(doc, "Getter docstring", "Getter docstring is not picked-up") - - doc = PropertySubWoDoc(getter, None, None, "issue 41287 is fixed").__doc__ - self.assertEqual(doc, "issue 41287 is fixed", - "Getter overrides explicit property docstring docstring") - - doc = PropertySubWoDoc(getter, None, None, None).__doc__ - self.assertEqual(doc, "Getter docstring", "Getter docstring is not picked-up") def getter_wo_doc(x): pass - doc = PropertySub(getter_wo_doc, None, None, "issue 41287 is fixed").__doc__ - self.assertEqual(doc, "issue 41287 is fixed", - "Getter overrides explicit property docstring docstring") - doc = PropertySub(getter_wo_doc, None, None, None).__doc__ - self.assertEqual(doc, "This is a subclass of property", - "Getter without docstring overrides PropertySub.__doc__") + for ps in PropertySub, PropertySubWoDoc: + doc = ps(getter, None, None, "issue 41287 is fixed").__doc__ + self.assertEqual(doc, "issue 41287 is fixed", + "Getter overrides explicit property docstring (%s)" % ps.__name__) - doc = PropertySubWoDoc(getter_wo_doc, None, None, "issue 41287 is fixed").__doc__ - self.assertEqual(doc, "issue 41287 is fixed", - "Getter overrides explicit property docstring docstring") + doc = ps(getter, None, None, None).__doc__ + self.assertEqual(doc, "Getter docstring", "Getter docstring is not picked-up (%s)" % ps.__name__) + + doc = ps(getter_wo_doc, None, None, "issue 41287 is fixed").__doc__ + self.assertEqual(doc, "issue 41287 is fixed", + "Getter overrides explicit property docstring (%s)" % ps.__name__) + + doc = ps(getter_wo_doc, None, None, None).__doc__ + self.assertIsNone(doc) @unittest.skipIf(sys.flags.optimize >= 2, "Docstrings are omitted with -O2 and above") From 009a9f3ef8799a53fda9b2bda235099f7cbbf32f Mon Sep 17 00:00:00 2001 From: Sergei Izmailov Date: Thu, 7 Apr 2022 23:21:12 +0300 Subject: [PATCH 09/17] Fix introduced refleak --- Objects/descrobject.c | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/Objects/descrobject.c b/Objects/descrobject.c index 92baf4eed52129..8e0b5f7d94cbfe 100644 --- a/Objects/descrobject.c +++ b/Objects/descrobject.c @@ -1791,33 +1791,33 @@ property_init_impl(propertyobject *self, PyObject *fget, PyObject *fset, self->getter_doc = 0; PyObject *prop_doc = NULL; + if (doc != NULL && doc != Py_None) { + prop_doc = doc; + Py_XINCREF(prop_doc); + } /* if no docstring given and the getter has one, use that one */ - if ((doc == NULL || doc == Py_None) && fget != NULL) { + else if (fget != NULL) { int rc = _PyObject_LookupAttr(fget, &_Py_ID(__doc__), &prop_doc); if (rc <= 0) { return rc; } self->getter_doc = 1; } - else { - prop_doc = doc; - Py_XINCREF(prop_doc); - } - - if (Py_IS_TYPE(self, &PyProperty_Type)) { - Py_XSETREF(self->prop_doc, prop_doc); - } - else if (prop_doc != NULL) { - /* If this is a property subclass, put __doc__ - in dict of the subclass instance instead, - otherwise it gets shadowed by __doc__ in the - class's dict. */ - int err = PyObject_SetAttr( - (PyObject *)self, &_Py_ID(__doc__), prop_doc); - Py_XDECREF(prop_doc); - if (err < 0) - return -1; + if (prop_doc != NULL && prop_doc != Py_None) { + if (Py_IS_TYPE(self, &PyProperty_Type)) { + Py_XSETREF(self->prop_doc, prop_doc); + } else { + /* If this is a property subclass, put __doc__ + in dict of the subclass instance instead, + otherwise it gets shadowed by __doc__ in the + class's dict. */ + int err = PyObject_SetAttr( + (PyObject *)self, &_Py_ID(__doc__), prop_doc); + Py_XDECREF(prop_doc); + if (err < 0) + return -1; + } } return 0; From 8730abc81abbae2023718a4a3a8af942e5416694 Mon Sep 17 00:00:00 2001 From: Sergei Izmailov Date: Sun, 22 May 2022 15:42:28 +0300 Subject: [PATCH 10/17] Test vanila property too --- Lib/test/test_property.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_property.py b/Lib/test/test_property.py index 532b2cdb275054..e4f71a68658e17 100644 --- a/Lib/test/test_property.py +++ b/Lib/test/test_property.py @@ -257,7 +257,7 @@ def getter(x): def getter_wo_doc(x): pass - for ps in PropertySub, PropertySubWoDoc: + for ps in property, PropertySub, PropertySubWoDoc: doc = ps(getter, None, None, "issue 41287 is fixed").__doc__ self.assertEqual(doc, "issue 41287 is fixed", "Getter overrides explicit property docstring (%s)" % ps.__name__) @@ -270,7 +270,7 @@ def getter_wo_doc(x): "Getter overrides explicit property docstring (%s)" % ps.__name__) doc = ps(getter_wo_doc, None, None, None).__doc__ - self.assertIsNone(doc) + self.assertIsNone(doc, "Property class doc appears in instance __doc__ (%s)" % ps.__name__) @unittest.skipIf(sys.flags.optimize >= 2, "Docstrings are omitted with -O2 and above") From 0302ffbf1e7cba30e8a646d9f340d321de4963c5 Mon Sep 17 00:00:00 2001 From: Sergei Izmailov Date: Sun, 22 May 2022 15:43:12 +0300 Subject: [PATCH 11/17] Fix __doc__ assignment in property subclass --- Objects/descrobject.c | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/Objects/descrobject.c b/Objects/descrobject.c index 8e0b5f7d94cbfe..8cf8064ab4c357 100644 --- a/Objects/descrobject.c +++ b/Objects/descrobject.c @@ -1804,20 +1804,25 @@ property_init_impl(propertyobject *self, PyObject *fget, PyObject *fset, self->getter_doc = 1; } - if (prop_doc != NULL && prop_doc != Py_None) { - if (Py_IS_TYPE(self, &PyProperty_Type)) { + if (Py_IS_TYPE(self, &PyProperty_Type)) { + if (prop_doc != NULL && prop_doc != Py_None) { Py_XSETREF(self->prop_doc, prop_doc); - } else { - /* If this is a property subclass, put __doc__ - in dict of the subclass instance instead, - otherwise it gets shadowed by __doc__ in the - class's dict. */ - int err = PyObject_SetAttr( - (PyObject *)self, &_Py_ID(__doc__), prop_doc); - Py_XDECREF(prop_doc); - if (err < 0) - return -1; } + } else { + /* If this is a property subclass, put __doc__ + in dict of the subclass instance instead, + otherwise it gets shadowed by __doc__ in the + class's dict. */ + + if (prop_doc == NULL) { + prop_doc = Py_None; + Py_INCREF(prop_doc); + } + int err = PyObject_SetAttr( + (PyObject *)self, &_Py_ID(__doc__), prop_doc); + Py_XDECREF(prop_doc); + if (err < 0) + return -1; } return 0; From fb8c2d4908d3f8ac992a404c2baa38090c33611e Mon Sep 17 00:00:00 2001 From: Sergei Izmailov Date: Sun, 22 May 2022 17:48:00 +0300 Subject: [PATCH 12/17] Fix Py_None ref leak --- Objects/descrobject.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Objects/descrobject.c b/Objects/descrobject.c index 3d0bdbdbdde61a..016a8530965901 100644 --- a/Objects/descrobject.c +++ b/Objects/descrobject.c @@ -1801,6 +1801,10 @@ property_init_impl(propertyobject *self, PyObject *fget, PyObject *fset, if (rc <= 0) { return rc; } + if (prop_doc == Py_None) { + prop_doc = NULL; + Py_DECREF(Py_None); + } self->getter_doc = 1; } From f8fb254ad15c11da45df45d8f4d0cab87172e58c Mon Sep 17 00:00:00 2001 From: Sergei Izmailov Date: Sun, 22 May 2022 21:15:13 +0300 Subject: [PATCH 13/17] Add a comment --- Objects/descrobject.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Objects/descrobject.c b/Objects/descrobject.c index 016a8530965901..bf19dd147a86a3 100644 --- a/Objects/descrobject.c +++ b/Objects/descrobject.c @@ -1808,6 +1808,9 @@ property_init_impl(propertyobject *self, PyObject *fget, PyObject *fset, self->getter_doc = 1; } + /* At this point `prop_doc` is either NULL or + a non-None object with incremented ref counter */ + if (Py_IS_TYPE(self, &PyProperty_Type)) { if (prop_doc != NULL && prop_doc != Py_None) { Py_XSETREF(self->prop_doc, prop_doc); From fc16ab974fb5d0e564478ed845ac18843bc70758 Mon Sep 17 00:00:00 2001 From: Sergei Izmailov Date: Sun, 22 May 2022 21:36:10 +0300 Subject: [PATCH 14/17] Remove unnecessary check --- Objects/descrobject.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Objects/descrobject.c b/Objects/descrobject.c index bf19dd147a86a3..6a6dfc9d50513c 100644 --- a/Objects/descrobject.c +++ b/Objects/descrobject.c @@ -1812,9 +1812,7 @@ property_init_impl(propertyobject *self, PyObject *fget, PyObject *fset, a non-None object with incremented ref counter */ if (Py_IS_TYPE(self, &PyProperty_Type)) { - if (prop_doc != NULL && prop_doc != Py_None) { - Py_XSETREF(self->prop_doc, prop_doc); - } + Py_XSETREF(self->prop_doc, prop_doc); } else { /* If this is a property subclass, put __doc__ in dict of the subclass instance instead, From 4943868df7fe79049c9b1e6443e44f993655f5ef Mon Sep 17 00:00:00 2001 From: Sergei Izmailov Date: Thu, 26 May 2022 21:46:25 +0300 Subject: [PATCH 15/17] Add test for property copy doc --- Lib/test/test_property.py | 60 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/Lib/test/test_property.py b/Lib/test/test_property.py index e4f71a68658e17..eac262dae92626 100644 --- a/Lib/test/test_property.py +++ b/Lib/test/test_property.py @@ -284,6 +284,66 @@ def spam(self): Foo.spam.__doc__, "spam wrapped in property subclass") + @unittest.skipIf(sys.flags.optimize >= 2, + "Docstrings are omitted with -O2 and above") + def test_docstring_copy2(self): + """ + Property tries to provide the best docstring it finds for it's copies. + If a user-provieded docstring is avaialble it's preserved in the copies. + If no docsting is available during property creation, the property + would utilze docstring from getter if available. + """ + def getter1(self): + return 1 + def getter2(self): + """doc 2""" + return 2 + def getter3(self): + """doc 3""" + return 3 + + # Case-1: user-provied doc is preserved in copies + # of property with undocumented getter + p = property(getter1, None, None, "doc-A") + + p2 = p.getter(getter2) + self.assertEqual(p.__doc__, "doc-A") + self.assertEqual(p2.__doc__, "doc-A") + + # Case-2: user-provied doc is preserved in copies + # of property with documented getter + p = property(getter2, None, None, "doc-A") + + p2 = p.getter(getter3) + self.assertEqual(p.__doc__, "doc-A") + self.assertEqual(p2.__doc__, "doc-A") + + # Case-3: with no user-provied doc new getter doc + # takes precendence + p = property(getter2, None, None, None) + + p2 = p.getter(getter3) + self.assertEqual(p.__doc__, "doc 2") + self.assertEqual(p2.__doc__, "doc 3") + + # Case-4: A user-provied doc is assigned after property construction + # with documented getter. The doc IS NOT preserved. + # It's an odd behaviour, but it's a strange enough + # use case with no easy solution. + p = property(getter2, None, None, None) + p.__doc__ = "user" + p2 = p.getter(getter3) + self.assertEqual(p.__doc__, "user") + self.assertEqual(p2.__doc__, "doc 3") + + # Case-5: A user-provied doc is assigned after property construction + # with UNdocumented getter. The doc IS preserved. + p = property(getter1, None, None, None) + p.__doc__ = "user" + p2 = p.getter(getter2) + self.assertEqual(p.__doc__, "user") + self.assertEqual(p2.__doc__, "user") + @unittest.skipIf(sys.flags.optimize >= 2, "Docstrings are omitted with -O2 and above") def test_property_setter_copies_getter_docstring(self): From 36fc1b09e78bb954b9f2d49e214749bac30e5085 Mon Sep 17 00:00:00 2001 From: Sergei Izmailov Date: Thu, 26 May 2022 21:47:35 +0300 Subject: [PATCH 16/17] Assign getter_doc only for non-null prop_doc --- Objects/descrobject.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Objects/descrobject.c b/Objects/descrobject.c index 6a6dfc9d50513c..c968885265a896 100644 --- a/Objects/descrobject.c +++ b/Objects/descrobject.c @@ -1805,7 +1805,9 @@ property_init_impl(propertyobject *self, PyObject *fget, PyObject *fset, prop_doc = NULL; Py_DECREF(Py_None); } - self->getter_doc = 1; + if (prop_doc != NULL){ + self->getter_doc = 1; + } } /* At this point `prop_doc` is either NULL or From 14bdd862dae123e5291ab3682831d0c765ef91f0 Mon Sep 17 00:00:00 2001 From: Sergei Izmailov Date: Sat, 28 May 2022 22:18:42 +0300 Subject: [PATCH 17/17] Apply suggestions from code review Fix typos in comments Co-authored-by: Jelle Zijlstra --- Lib/test/test_property.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/Lib/test/test_property.py b/Lib/test/test_property.py index eac262dae92626..d07b8632aa8722 100644 --- a/Lib/test/test_property.py +++ b/Lib/test/test_property.py @@ -288,10 +288,10 @@ def spam(self): "Docstrings are omitted with -O2 and above") def test_docstring_copy2(self): """ - Property tries to provide the best docstring it finds for it's copies. - If a user-provieded docstring is avaialble it's preserved in the copies. - If no docsting is available during property creation, the property - would utilze docstring from getter if available. + Property tries to provide the best docstring it finds for its instances. + If a user-provided docstring is available, it is preserved on copies. + If no docstring is available during property creation, the property + will utilize the docstring from the getter if available. """ def getter1(self): return 1 @@ -302,7 +302,7 @@ def getter3(self): """doc 3""" return 3 - # Case-1: user-provied doc is preserved in copies + # Case-1: user-provided doc is preserved in copies # of property with undocumented getter p = property(getter1, None, None, "doc-A") @@ -310,7 +310,7 @@ def getter3(self): self.assertEqual(p.__doc__, "doc-A") self.assertEqual(p2.__doc__, "doc-A") - # Case-2: user-provied doc is preserved in copies + # Case-2: user-provided doc is preserved in copies # of property with documented getter p = property(getter2, None, None, "doc-A") @@ -318,7 +318,7 @@ def getter3(self): self.assertEqual(p.__doc__, "doc-A") self.assertEqual(p2.__doc__, "doc-A") - # Case-3: with no user-provied doc new getter doc + # Case-3: with no user-provided doc new getter doc # takes precendence p = property(getter2, None, None, None) @@ -326,7 +326,7 @@ def getter3(self): self.assertEqual(p.__doc__, "doc 2") self.assertEqual(p2.__doc__, "doc 3") - # Case-4: A user-provied doc is assigned after property construction + # Case-4: A user-provided doc is assigned after property construction # with documented getter. The doc IS NOT preserved. # It's an odd behaviour, but it's a strange enough # use case with no easy solution. @@ -336,7 +336,7 @@ def getter3(self): self.assertEqual(p.__doc__, "user") self.assertEqual(p2.__doc__, "doc 3") - # Case-5: A user-provied doc is assigned after property construction + # Case-5: A user-provided doc is assigned after property construction # with UNdocumented getter. The doc IS preserved. p = property(getter1, None, None, None) p.__doc__ = "user"