From a840465368ca5f9ab414060a91c7147dc19721ec Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Sat, 1 Apr 2023 10:23:01 +0300 Subject: [PATCH 01/17] gh-106016: Support customizing of module attributes access with __setattr__/__delattr__ --- Doc/reference/datamodel.rst | 50 ++++++++++++------- Lib/test/bad_delattr.py | 2 + Lib/test/bad_delattr2.py | 4 ++ Lib/test/bad_setattr.py | 2 + Lib/test/bad_setattr2.py | 4 ++ Lib/test/good_delattr.py | 6 +++ Lib/test/good_setattr.py | 6 +++ Lib/test/test_module/__init__.py | 47 +++++++++++++++++ ...-08-22-09-57-59.gh-issue-106016.lpbUq8.rst | 2 + Objects/moduleobject.c | 36 ++++++++++++- 10 files changed, 141 insertions(+), 18 deletions(-) create mode 100644 Lib/test/bad_delattr.py create mode 100644 Lib/test/bad_delattr2.py create mode 100644 Lib/test/bad_setattr.py create mode 100644 Lib/test/bad_setattr2.py create mode 100644 Lib/test/good_delattr.py create mode 100644 Lib/test/good_setattr.py create mode 100644 Misc/NEWS.d/next/Core and Builtins/2023-08-22-09-57-59.gh-issue-106016.lpbUq8.rst diff --git a/Doc/reference/datamodel.rst b/Doc/reference/datamodel.rst index 229fa696c9142f..5a866658f0eb21 100644 --- a/Doc/reference/datamodel.rst +++ b/Doc/reference/datamodel.rst @@ -1695,11 +1695,15 @@ Customizing module attribute access .. index:: single: __getattr__ (module attribute) + single: __setattr__ (module attribute) + single: __delattr__ (module attribute) single: __dir__ (module attribute) single: __class__ (module attribute) -Special names ``__getattr__`` and ``__dir__`` can be also used to customize -access to module attributes. The ``__getattr__`` function at the module level +Special names ``__getattr__``, ``__setattr__``, ``__delattr__`` and ``__dir__`` +can be also used to customize access to module attributes. + +The ``__getattr__`` function at the module level should accept one argument which is the name of an attribute and return the computed value or raise an :exc:`AttributeError`. If an attribute is not found on a module object through the normal lookup, i.e. @@ -1707,32 +1711,41 @@ not found on a module object through the normal lookup, i.e. the module ``__dict__`` before raising an :exc:`AttributeError`. If found, it is called with the attribute name and the result is returned. +The ``__setattr__`` function should accept two arguments, respectively, the +name of an attribute and the value to be assigned and return :const:`None` or +raise an :exc:`AttributeError`. If present, this function overrides the +standard :func:`setattr` behaviour on a module. For example:: + + def __setattr__(name, value): + print(f'Setting {name} to {value}...') + globals()[name] = value + +The ``__delattr__`` function should accept one argument which is the name of an +attribute and return :const:`None` or raise an :exc:`AttributeError`. If present, +this function overrides the standard :func:`delattr` behaviour on a module. + The ``__dir__`` function should accept no arguments, and return a sequence of strings that represents the names accessible on module. If present, this function overrides the standard :func:`dir` search on a module. -For a more fine grained customization of the module behavior (setting -attributes, properties, etc.), one can set the ``__class__`` attribute of -a module object to a subclass of :class:`types.ModuleType`. For example:: +For a more fine grained customization of the module behavior, one can set the +``__class__`` attribute of a module object to a subclass of +:class:`types.ModuleType`. For example:: import sys from types import ModuleType - class VerboseModule(ModuleType): - def __repr__(self): - return f'Verbose {self.__name__}' - - def __setattr__(self, attr, value): - print(f'Setting {attr}...') - super().__setattr__(attr, value) + class CallableModule(ModuleType): + def __call__(self, *args, **kwargs): + raise RuntimeError("NO-body expects the Spanish Inquisition!") - sys.modules[__name__].__class__ = VerboseModule + sys.modules[__name__].__class__ = CallableModule .. note:: - Defining module ``__getattr__`` and setting module ``__class__`` only - affect lookups made using the attribute access syntax -- directly accessing - the module globals (whether by code within the module, or via a reference - to the module's globals dictionary) is unaffected. + Defining module ``__getattr__``/``__setattr__``/``__delattr__`` and setting + module ``__class__`` only affect lookups made using the attribute access + syntax -- directly accessing the module globals (whether by code within the + module, or via a reference to the module's globals dictionary) is unaffected. .. versionchanged:: 3.5 ``__class__`` module attribute is now writable. @@ -1740,6 +1753,9 @@ a module object to a subclass of :class:`types.ModuleType`. For example:: .. versionadded:: 3.7 ``__getattr__`` and ``__dir__`` module attributes. +.. versionadded:: 3.13 + ``__setattr__`` and ``__delattr__`` module attributes. + .. seealso:: :pep:`562` - Module __getattr__ and __dir__ diff --git a/Lib/test/bad_delattr.py b/Lib/test/bad_delattr.py new file mode 100644 index 00000000000000..5919b7dda26003 --- /dev/null +++ b/Lib/test/bad_delattr.py @@ -0,0 +1,2 @@ +foo = 1 +__delattr__ = "Oops" diff --git a/Lib/test/bad_delattr2.py b/Lib/test/bad_delattr2.py new file mode 100644 index 00000000000000..93fc7a21774f8c --- /dev/null +++ b/Lib/test/bad_delattr2.py @@ -0,0 +1,4 @@ +foo = 1 + +def __delattr__(): + "Bad function signature" diff --git a/Lib/test/bad_setattr.py b/Lib/test/bad_setattr.py new file mode 100644 index 00000000000000..6b30f22f7715e6 --- /dev/null +++ b/Lib/test/bad_setattr.py @@ -0,0 +1,2 @@ +foo = 1 +__setattr__ = "Oops" diff --git a/Lib/test/bad_setattr2.py b/Lib/test/bad_setattr2.py new file mode 100644 index 00000000000000..efa28456b5e9c2 --- /dev/null +++ b/Lib/test/bad_setattr2.py @@ -0,0 +1,4 @@ +foo = 1 + +def __setattr__(): + "Bad function signature" diff --git a/Lib/test/good_delattr.py b/Lib/test/good_delattr.py new file mode 100644 index 00000000000000..97b29d68956c93 --- /dev/null +++ b/Lib/test/good_delattr.py @@ -0,0 +1,6 @@ +foo = 1 + +def __delattr__(name): + if name == 'foo': + raise AttributeError("Read-only attribute") + del globals()[name] diff --git a/Lib/test/good_setattr.py b/Lib/test/good_setattr.py new file mode 100644 index 00000000000000..4c28d05bbbdaf0 --- /dev/null +++ b/Lib/test/good_setattr.py @@ -0,0 +1,6 @@ +foo = 1 + +def __setattr__(name, value): + if name == 'foo': + raise AttributeError("Read-only attribute") + globals()[name] = value diff --git a/Lib/test/test_module/__init__.py b/Lib/test/test_module/__init__.py index cfc4d9ccf1cc81..b85ffc7f0a5ad1 100644 --- a/Lib/test/test_module/__init__.py +++ b/Lib/test/test_module/__init__.py @@ -150,6 +150,53 @@ def test_module_getattr_errors(self): if 'test.test_module.bad_getattr2' in sys.modules: del sys.modules['test.test_module.bad_getattr2'] + def test_module_setattr(self): + import test.good_setattr as gsa + self.assertEqual(gsa.foo, 1) + with self.assertRaises(AttributeError): + gsa.bar + with self.assertRaisesRegex(AttributeError, "Read-only attribute"): + gsa.foo = 2 + gsa.bar = 3 + self.assertEqual(gsa.bar, 3) + + def test_module_setattr_errors(self): + import test.bad_setattr as bsa + from test import bad_setattr2 + self.assertEqual(bsa.foo, 1) + self.assertEqual(bad_setattr2.foo, 1) + with self.assertRaises(TypeError): + bsa.foo = 2 + with self.assertRaises(TypeError): + bad_setattr2.foo = 2 + del sys.modules['test.bad_setattr'] + del sys.modules['test.bad_setattr2'] + + def test_module_delattr(self): + import test.good_delattr as gda + self.assertEqual(gda.foo, 1) + with self.assertRaises(AttributeError): + gda.bar + with self.assertRaisesRegex(AttributeError, "Read-only attribute"): + del gda.foo + gda.bar = 3 + self.assertEqual(gda.bar, 3) + del gda.bar + with self.assertRaises(AttributeError): + gda.bar + + def test_module_delattr_errors(self): + import test.bad_delattr as bda + from test import bad_delattr2 + self.assertEqual(bda.foo, 1) + self.assertEqual(bad_delattr2.foo, 1) + with self.assertRaises(TypeError): + del bda.foo + with self.assertRaises(TypeError): + del bad_delattr2.foo + del sys.modules['test.bad_delattr'] + del sys.modules['test.bad_delattr2'] + def test_module_dir(self): import test.test_module.good_getattr as gga self.assertEqual(dir(gga), ['a', 'b', 'c']) diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-08-22-09-57-59.gh-issue-106016.lpbUq8.rst b/Misc/NEWS.d/next/Core and Builtins/2023-08-22-09-57-59.gh-issue-106016.lpbUq8.rst new file mode 100644 index 00000000000000..17a126a5bbd592 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2023-08-22-09-57-59.gh-issue-106016.lpbUq8.rst @@ -0,0 +1,2 @@ +Add support for module ``__setattr__`` and ``__delattr__``. Patch by Sergey +B Kirpichev. diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index 7e890d021cb946..abf95211c50f8c 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -866,6 +866,40 @@ _Py_module_getattro(PyModuleObject *m, PyObject *name) return _Py_module_getattro_impl(m, name, 0); } +static int +module_setattro(PyModuleObject *m, PyObject *name, PyObject *value) +{ + PyObject *setattr, *delattr; + assert(m->md_dict != NULL); + PyErr_Clear(); + if (value == NULL) { + delattr = PyDict_GetItemWithError(m->md_dict, &_Py_ID(__delattr__)); + if (PyErr_Occurred()) { + return -1; + } + if (delattr) { + PyObject_CallFunctionObjArgs(delattr, name, NULL); + if (PyErr_Occurred()) { + return -1; + } + return 0; + } + } else { + setattr = PyDict_GetItemWithError(m->md_dict, &_Py_ID(__setattr__)); + if (PyErr_Occurred()) { + return -1; + } + if (setattr) { + PyObject_CallFunctionObjArgs(setattr, name, value, NULL); + if (PyErr_Occurred()) { + return -1; + } + return 0; + } + } + return PyObject_GenericSetAttr((PyObject *)m, name, value); +} + static int module_traverse(PyModuleObject *m, visitproc visit, void *arg) { @@ -1018,7 +1052,7 @@ PyTypeObject PyModule_Type = { 0, /* tp_call */ 0, /* tp_str */ (getattrofunc)_Py_module_getattro, /* tp_getattro */ - PyObject_GenericSetAttr, /* tp_setattro */ + (setattrofunc)module_setattro, /* tp_setattro */ 0, /* tp_as_buffer */ Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_BASETYPE, /* tp_flags */ From 3e78225b8847e195f15a027a8b9a4428681156ca Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Tue, 22 Aug 2023 16:03:06 +0300 Subject: [PATCH 02/17] Update Doc/reference/datamodel.rst Co-authored-by: Victor Stinner --- Doc/reference/datamodel.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Doc/reference/datamodel.rst b/Doc/reference/datamodel.rst index 5a866658f0eb21..e769820a75602b 100644 --- a/Doc/reference/datamodel.rst +++ b/Doc/reference/datamodel.rst @@ -1742,8 +1742,8 @@ For a more fine grained customization of the module behavior, one can set the sys.modules[__name__].__class__ = CallableModule .. note:: - Defining module ``__getattr__``/``__setattr__``/``__delattr__`` and setting - module ``__class__`` only affect lookups made using the attribute access + Defining module ``__getattr__``, ``__setattr__``, ``__delattr__``, ``__dir__`` + and setting module ``__class__`` only affect lookups made using the attribute access syntax -- directly accessing the module globals (whether by code within the module, or via a reference to the module's globals dictionary) is unaffected. From 0a8cec616c743a670b6ef91bc51e52a1998e23b9 Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Tue, 22 Aug 2023 16:03:22 +0300 Subject: [PATCH 03/17] Update Doc/reference/datamodel.rst Co-authored-by: Victor Stinner --- Doc/reference/datamodel.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Doc/reference/datamodel.rst b/Doc/reference/datamodel.rst index e769820a75602b..c562c8d967de28 100644 --- a/Doc/reference/datamodel.rst +++ b/Doc/reference/datamodel.rst @@ -1712,7 +1712,7 @@ the module ``__dict__`` before raising an :exc:`AttributeError`. If found, it is called with the attribute name and the result is returned. The ``__setattr__`` function should accept two arguments, respectively, the -name of an attribute and the value to be assigned and return :const:`None` or +name of an attribute and the value to be assigned. It can raise an :exc:`AttributeError`. If present, this function overrides the standard :func:`setattr` behaviour on a module. For example:: From 2904f501f23d9259997aba74ffa9f0a41b43cf31 Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Tue, 22 Aug 2023 16:03:41 +0300 Subject: [PATCH 04/17] Update Doc/reference/datamodel.rst Co-authored-by: Victor Stinner --- Doc/reference/datamodel.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Doc/reference/datamodel.rst b/Doc/reference/datamodel.rst index c562c8d967de28..1882b46547fc82 100644 --- a/Doc/reference/datamodel.rst +++ b/Doc/reference/datamodel.rst @@ -1721,7 +1721,7 @@ standard :func:`setattr` behaviour on a module. For example:: globals()[name] = value The ``__delattr__`` function should accept one argument which is the name of an -attribute and return :const:`None` or raise an :exc:`AttributeError`. If present, +attribute. It can raise an :exc:`AttributeError`. If present, this function overrides the standard :func:`delattr` behaviour on a module. The ``__dir__`` function should accept no arguments, and return a sequence of From bce16674cc68f31e779120452792d64e96388518 Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Tue, 22 Aug 2023 16:09:42 +0300 Subject: [PATCH 05/17] Drop PyErr_Clear() --- Objects/moduleobject.c | 1 - 1 file changed, 1 deletion(-) diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index abf95211c50f8c..a1683171bf9729 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -871,7 +871,6 @@ module_setattro(PyModuleObject *m, PyObject *name, PyObject *value) { PyObject *setattr, *delattr; assert(m->md_dict != NULL); - PyErr_Clear(); if (value == NULL) { delattr = PyDict_GetItemWithError(m->md_dict, &_Py_ID(__delattr__)); if (PyErr_Occurred()) { From bcafd0d329e1ff9efa63c0ca41673a684a441acd Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Tue, 22 Aug 2023 16:11:30 +0300 Subject: [PATCH 06/17] move delattr/setattr declarations --- Objects/moduleobject.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index a1683171bf9729..5219fa34576e74 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -869,10 +869,9 @@ _Py_module_getattro(PyModuleObject *m, PyObject *name) static int module_setattro(PyModuleObject *m, PyObject *name, PyObject *value) { - PyObject *setattr, *delattr; assert(m->md_dict != NULL); if (value == NULL) { - delattr = PyDict_GetItemWithError(m->md_dict, &_Py_ID(__delattr__)); + PyObject *delattr = PyDict_GetItemWithError(m->md_dict, &_Py_ID(__delattr__)); if (PyErr_Occurred()) { return -1; } @@ -884,7 +883,7 @@ module_setattro(PyModuleObject *m, PyObject *name, PyObject *value) return 0; } } else { - setattr = PyDict_GetItemWithError(m->md_dict, &_Py_ID(__setattr__)); + PyObject *setattr = PyDict_GetItemWithError(m->md_dict, &_Py_ID(__setattr__)); if (PyErr_Occurred()) { return -1; } From 0740efb78f2c4aff4991bdb0f25acf46847bce5a Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Tue, 22 Aug 2023 16:12:48 +0300 Subject: [PATCH 07/17] m -> mod --- Objects/moduleobject.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index 5219fa34576e74..572e0db2d755c9 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -867,11 +867,11 @@ _Py_module_getattro(PyModuleObject *m, PyObject *name) } static int -module_setattro(PyModuleObject *m, PyObject *name, PyObject *value) +module_setattro(PyModuleObject *mod, PyObject *name, PyObject *value) { - assert(m->md_dict != NULL); + assert(mod->md_dict != NULL); if (value == NULL) { - PyObject *delattr = PyDict_GetItemWithError(m->md_dict, &_Py_ID(__delattr__)); + PyObject *delattr = PyDict_GetItemWithError(mod->md_dict, &_Py_ID(__delattr__)); if (PyErr_Occurred()) { return -1; } @@ -883,7 +883,7 @@ module_setattro(PyModuleObject *m, PyObject *name, PyObject *value) return 0; } } else { - PyObject *setattr = PyDict_GetItemWithError(m->md_dict, &_Py_ID(__setattr__)); + PyObject *setattr = PyDict_GetItemWithError(mod->md_dict, &_Py_ID(__setattr__)); if (PyErr_Occurred()) { return -1; } @@ -895,7 +895,7 @@ module_setattro(PyModuleObject *m, PyObject *name, PyObject *value) return 0; } } - return PyObject_GenericSetAttr((PyObject *)m, name, value); + return PyObject_GenericSetAttr((PyObject *)mod, name, value); } static int From 74c3d138ce75d08470e0e76571ce77424c3597c1 Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Tue, 22 Aug 2023 16:19:50 +0300 Subject: [PATCH 08/17] Check PyObject_CallFunctionObjArgs return value --- Objects/moduleobject.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index 572e0db2d755c9..3cc316e8fc6709 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -869,6 +869,7 @@ _Py_module_getattro(PyModuleObject *m, PyObject *name) static int module_setattro(PyModuleObject *mod, PyObject *name, PyObject *value) { + PyObject *ret; assert(mod->md_dict != NULL); if (value == NULL) { PyObject *delattr = PyDict_GetItemWithError(mod->md_dict, &_Py_ID(__delattr__)); @@ -876,8 +877,8 @@ module_setattro(PyModuleObject *mod, PyObject *name, PyObject *value) return -1; } if (delattr) { - PyObject_CallFunctionObjArgs(delattr, name, NULL); - if (PyErr_Occurred()) { + ret = PyObject_CallFunctionObjArgs(delattr, name, NULL); + if (ret == NULL) { return -1; } return 0; @@ -888,8 +889,8 @@ module_setattro(PyModuleObject *mod, PyObject *name, PyObject *value) return -1; } if (setattr) { - PyObject_CallFunctionObjArgs(setattr, name, value, NULL); - if (PyErr_Occurred()) { + ret = PyObject_CallFunctionObjArgs(setattr, name, value, NULL); + if (ret == NULL) { return -1; } return 0; From 83c4c504dca2ee1e78c685f4507ac79f71c6bcce Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Tue, 22 Aug 2023 16:32:26 +0300 Subject: [PATCH 09/17] Document changes in the "Other Language Changes" section --- Doc/whatsnew/3.13.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Doc/whatsnew/3.13.rst b/Doc/whatsnew/3.13.rst index 8509e18a7d792e..0139b4284e640b 100644 --- a/Doc/whatsnew/3.13.rst +++ b/Doc/whatsnew/3.13.rst @@ -91,6 +91,10 @@ Other Language Changes of the ``optimize`` argument. (Contributed by Irit Katriel in :gh:`108113`). +* Support customizing module attribute access with ``__setattr__`` and + ``__delattr__`` functions at the module level. (Contributed by + Sergey B Kirpichev in :gh:`108261`). + New Modules =========== From 7fe6a06aa2070886958e8c596fdcb5df74280fca Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Tue, 22 Aug 2023 17:19:03 +0300 Subject: [PATCH 10/17] Use PyDict_GetItemRef instead of PyDict_GetItemWithError --- Objects/moduleobject.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index 3cc316e8fc6709..fad39560b43fca 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -869,28 +869,31 @@ _Py_module_getattro(PyModuleObject *m, PyObject *name) static int module_setattro(PyModuleObject *mod, PyObject *name, PyObject *value) { - PyObject *ret; + PyObject *res; + int ret; assert(mod->md_dict != NULL); if (value == NULL) { - PyObject *delattr = PyDict_GetItemWithError(mod->md_dict, &_Py_ID(__delattr__)); - if (PyErr_Occurred()) { + PyObject *delattr; + ret = PyDict_GetItemRef(mod->md_dict, &_Py_ID(__delattr__), &delattr); + if (ret == -1) { return -1; } - if (delattr) { - ret = PyObject_CallFunctionObjArgs(delattr, name, NULL); - if (ret == NULL) { + if (ret) { + res = PyObject_CallFunctionObjArgs(delattr, name, NULL); + if (res == NULL) { return -1; } return 0; } } else { - PyObject *setattr = PyDict_GetItemWithError(mod->md_dict, &_Py_ID(__setattr__)); - if (PyErr_Occurred()) { + PyObject *setattr; + ret = PyDict_GetItemRef(mod->md_dict, &_Py_ID(__setattr__), &setattr); + if (ret == -1) { return -1; } - if (setattr) { - ret = PyObject_CallFunctionObjArgs(setattr, name, value, NULL); - if (ret == NULL) { + if (ret) { + res = PyObject_CallFunctionObjArgs(setattr, name, value, NULL); + if (res == NULL) { return -1; } return 0; From 47d93ce7d16c47510cd9719f468262f1c3ae4956 Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Tue, 22 Aug 2023 18:03:59 +0300 Subject: [PATCH 11/17] Move test modules to Lib/test/test_module/ --- Lib/test/test_module/__init__.py | 20 ++++++++++---------- Lib/test/{ => test_module}/bad_delattr.py | 0 Lib/test/{ => test_module}/bad_delattr2.py | 0 Lib/test/{ => test_module}/bad_setattr.py | 0 Lib/test/{ => test_module}/bad_setattr2.py | 0 Lib/test/{ => test_module}/good_delattr.py | 0 Lib/test/{ => test_module}/good_setattr.py | 0 7 files changed, 10 insertions(+), 10 deletions(-) rename Lib/test/{ => test_module}/bad_delattr.py (100%) rename Lib/test/{ => test_module}/bad_delattr2.py (100%) rename Lib/test/{ => test_module}/bad_setattr.py (100%) rename Lib/test/{ => test_module}/bad_setattr2.py (100%) rename Lib/test/{ => test_module}/good_delattr.py (100%) rename Lib/test/{ => test_module}/good_setattr.py (100%) diff --git a/Lib/test/test_module/__init__.py b/Lib/test/test_module/__init__.py index b85ffc7f0a5ad1..9e0458fb3b5c20 100644 --- a/Lib/test/test_module/__init__.py +++ b/Lib/test/test_module/__init__.py @@ -151,7 +151,7 @@ def test_module_getattr_errors(self): del sys.modules['test.test_module.bad_getattr2'] def test_module_setattr(self): - import test.good_setattr as gsa + import test.test_module.good_setattr as gsa self.assertEqual(gsa.foo, 1) with self.assertRaises(AttributeError): gsa.bar @@ -161,19 +161,19 @@ def test_module_setattr(self): self.assertEqual(gsa.bar, 3) def test_module_setattr_errors(self): - import test.bad_setattr as bsa - from test import bad_setattr2 + import test.test_module.bad_setattr as bsa + from test.test_module import bad_setattr2 self.assertEqual(bsa.foo, 1) self.assertEqual(bad_setattr2.foo, 1) with self.assertRaises(TypeError): bsa.foo = 2 with self.assertRaises(TypeError): bad_setattr2.foo = 2 - del sys.modules['test.bad_setattr'] - del sys.modules['test.bad_setattr2'] + del sys.modules['test.test_module.bad_setattr'] + del sys.modules['test.test_module.bad_setattr2'] def test_module_delattr(self): - import test.good_delattr as gda + import test.test_module.good_delattr as gda self.assertEqual(gda.foo, 1) with self.assertRaises(AttributeError): gda.bar @@ -186,16 +186,16 @@ def test_module_delattr(self): gda.bar def test_module_delattr_errors(self): - import test.bad_delattr as bda - from test import bad_delattr2 + import test.test_module.bad_delattr as bda + from test.test_module import bad_delattr2 self.assertEqual(bda.foo, 1) self.assertEqual(bad_delattr2.foo, 1) with self.assertRaises(TypeError): del bda.foo with self.assertRaises(TypeError): del bad_delattr2.foo - del sys.modules['test.bad_delattr'] - del sys.modules['test.bad_delattr2'] + del sys.modules['test.test_module.bad_delattr'] + del sys.modules['test.test_module.bad_delattr2'] def test_module_dir(self): import test.test_module.good_getattr as gga diff --git a/Lib/test/bad_delattr.py b/Lib/test/test_module/bad_delattr.py similarity index 100% rename from Lib/test/bad_delattr.py rename to Lib/test/test_module/bad_delattr.py diff --git a/Lib/test/bad_delattr2.py b/Lib/test/test_module/bad_delattr2.py similarity index 100% rename from Lib/test/bad_delattr2.py rename to Lib/test/test_module/bad_delattr2.py diff --git a/Lib/test/bad_setattr.py b/Lib/test/test_module/bad_setattr.py similarity index 100% rename from Lib/test/bad_setattr.py rename to Lib/test/test_module/bad_setattr.py diff --git a/Lib/test/bad_setattr2.py b/Lib/test/test_module/bad_setattr2.py similarity index 100% rename from Lib/test/bad_setattr2.py rename to Lib/test/test_module/bad_setattr2.py diff --git a/Lib/test/good_delattr.py b/Lib/test/test_module/good_delattr.py similarity index 100% rename from Lib/test/good_delattr.py rename to Lib/test/test_module/good_delattr.py diff --git a/Lib/test/good_setattr.py b/Lib/test/test_module/good_setattr.py similarity index 100% rename from Lib/test/good_setattr.py rename to Lib/test/test_module/good_setattr.py From f3f796dbb0d6962c2d1ae30ff052f599793f12c5 Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Wed, 23 Aug 2023 05:58:46 +0300 Subject: [PATCH 12/17] Avoid ret variable --- Objects/moduleobject.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index fad39560b43fca..dff8184027c1c7 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -870,15 +870,13 @@ static int module_setattro(PyModuleObject *mod, PyObject *name, PyObject *value) { PyObject *res; - int ret; assert(mod->md_dict != NULL); if (value == NULL) { PyObject *delattr; - ret = PyDict_GetItemRef(mod->md_dict, &_Py_ID(__delattr__), &delattr); - if (ret == -1) { + if (PyDict_GetItemRef(mod->md_dict, &_Py_ID(__delattr__), &delattr) < 0) { return -1; } - if (ret) { + if (delattr) { res = PyObject_CallFunctionObjArgs(delattr, name, NULL); if (res == NULL) { return -1; @@ -887,11 +885,10 @@ module_setattro(PyModuleObject *mod, PyObject *name, PyObject *value) } } else { PyObject *setattr; - ret = PyDict_GetItemRef(mod->md_dict, &_Py_ID(__setattr__), &setattr); - if (ret == -1) { + if (PyDict_GetItemRef(mod->md_dict, &_Py_ID(__setattr__), &setattr) < 0) { return -1; } - if (ret) { + if (setattr) { res = PyObject_CallFunctionObjArgs(setattr, name, value, NULL); if (res == NULL) { return -1; From f971a228d91436978fc218912d3696d9e1b4868f Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Wed, 23 Aug 2023 06:04:43 +0300 Subject: [PATCH 13/17] +Py_DECREF --- Objects/moduleobject.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index dff8184027c1c7..76f2dbd3656e55 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -878,9 +878,11 @@ module_setattro(PyModuleObject *mod, PyObject *name, PyObject *value) } if (delattr) { res = PyObject_CallFunctionObjArgs(delattr, name, NULL); + Py_DECREF(delattr); if (res == NULL) { return -1; } + Py_DECREF(res); return 0; } } else { @@ -890,9 +892,11 @@ module_setattro(PyModuleObject *mod, PyObject *name, PyObject *value) } if (setattr) { res = PyObject_CallFunctionObjArgs(setattr, name, value, NULL); + Py_DECREF(setattr); if (res == NULL) { return -1; } + Py_DECREF(res); return 0; } } From 68eefbdcf994f8a5c5bc0c1e09ce873b407ee957 Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Wed, 23 Aug 2023 06:25:20 +0300 Subject: [PATCH 14/17] Reorder setattr/delattr paths --- Objects/moduleobject.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index 76f2dbd3656e55..f1bc35efe5db0b 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -869,16 +869,15 @@ _Py_module_getattro(PyModuleObject *m, PyObject *name) static int module_setattro(PyModuleObject *mod, PyObject *name, PyObject *value) { - PyObject *res; assert(mod->md_dict != NULL); - if (value == NULL) { - PyObject *delattr; - if (PyDict_GetItemRef(mod->md_dict, &_Py_ID(__delattr__), &delattr) < 0) { + if (value) { + PyObject *setattr; + if (PyDict_GetItemRef(mod->md_dict, &_Py_ID(__setattr__), &setattr) < 0) { return -1; } - if (delattr) { - res = PyObject_CallFunctionObjArgs(delattr, name, NULL); - Py_DECREF(delattr); + if (setattr) { + PyObject *res = PyObject_CallFunctionObjArgs(setattr, name, value, NULL); + Py_DECREF(setattr); if (res == NULL) { return -1; } @@ -886,13 +885,13 @@ module_setattro(PyModuleObject *mod, PyObject *name, PyObject *value) return 0; } } else { - PyObject *setattr; - if (PyDict_GetItemRef(mod->md_dict, &_Py_ID(__setattr__), &setattr) < 0) { + PyObject *delattr; + if (PyDict_GetItemRef(mod->md_dict, &_Py_ID(__delattr__), &delattr) < 0) { return -1; } - if (setattr) { - res = PyObject_CallFunctionObjArgs(setattr, name, value, NULL); - Py_DECREF(setattr); + if (delattr) { + PyObject *res = PyObject_CallFunctionObjArgs(delattr, name, NULL); + Py_DECREF(delattr); if (res == NULL) { return -1; } From 058412eb00edf5bdd4718be1d5a197f80de70e3c Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Thu, 24 Aug 2023 03:03:21 +0300 Subject: [PATCH 15/17] Apply suggestions from code review Co-authored-by: Victor Stinner --- Lib/test/test_module/__init__.py | 7 +++++-- Objects/moduleobject.c | 3 ++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_module/__init__.py b/Lib/test/test_module/__init__.py index 9e0458fb3b5c20..b6c8f9f892fca0 100644 --- a/Lib/test/test_module/__init__.py +++ b/Lib/test/test_module/__init__.py @@ -159,6 +159,7 @@ def test_module_setattr(self): gsa.foo = 2 gsa.bar = 3 self.assertEqual(gsa.bar, 3) + del sys.modules['test.test_module.good_setattr'] def test_module_setattr_errors(self): import test.test_module.bad_setattr as bsa @@ -170,7 +171,8 @@ def test_module_setattr_errors(self): with self.assertRaises(TypeError): bad_setattr2.foo = 2 del sys.modules['test.test_module.bad_setattr'] - del sys.modules['test.test_module.bad_setattr2'] + if 'test.test_module.bad_setattr2' in sys.modules: + del sys.modules['test.test_module.bad_setattr2'] def test_module_delattr(self): import test.test_module.good_delattr as gda @@ -195,7 +197,8 @@ def test_module_delattr_errors(self): with self.assertRaises(TypeError): del bad_delattr2.foo del sys.modules['test.test_module.bad_delattr'] - del sys.modules['test.test_module.bad_delattr2'] + if 'test.test_module.bad_delattr2' in sys.modules: + del sys.modules['test.test_module.bad_delattr2'] def test_module_dir(self): import test.test_module.good_getattr as gga diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index f1bc35efe5db0b..22bf561ce7aed2 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -884,7 +884,8 @@ module_setattro(PyModuleObject *mod, PyObject *name, PyObject *value) Py_DECREF(res); return 0; } - } else { + } + else { PyObject *delattr; if (PyDict_GetItemRef(mod->md_dict, &_Py_ID(__delattr__), &delattr) < 0) { return -1; From 49c82c5600f9bbdb09afa6143d0a1f05356b9730 Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Thu, 24 Aug 2023 04:07:52 +0300 Subject: [PATCH 16/17] Update Lib/test/test_module/__init__.py --- Lib/test/test_module/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/Lib/test/test_module/__init__.py b/Lib/test/test_module/__init__.py index b6c8f9f892fca0..7018cd8304a13d 100644 --- a/Lib/test/test_module/__init__.py +++ b/Lib/test/test_module/__init__.py @@ -186,6 +186,7 @@ def test_module_delattr(self): del gda.bar with self.assertRaises(AttributeError): gda.bar + del sys.modules['test.test_module.good_delattr'] def test_module_delattr_errors(self): import test.test_module.bad_delattr as bda From cd0e4bef01c6b1c43d0fe9b9a63f536eec715266 Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Sun, 25 Feb 2024 11:36:50 +0300 Subject: [PATCH 17/17] Mention PEP in docs, like PEP 562 --- Doc/reference/datamodel.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Doc/reference/datamodel.rst b/Doc/reference/datamodel.rst index 0d7a743c61a034..b1f23fb953258e 100644 --- a/Doc/reference/datamodel.rst +++ b/Doc/reference/datamodel.rst @@ -2073,6 +2073,9 @@ For a more fine grained customization of the module behavior, one can set the :pep:`562` - Module __getattr__ and __dir__ Describes the ``__getattr__`` and ``__dir__`` functions on modules. + :pep:`726` - Module __setattr__ and __delattr__ + Proposes the ``__setattr__`` and ``__delattr__`` functions on modules. + .. _descriptors: