Skip to content

PEP 726: Support defining __setattr__() and __delattr__() in a module #106016

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
vstinner opened this issue Jun 23, 2023 · 8 comments
Closed

PEP 726: Support defining __setattr__() and __delattr__() in a module #106016

vstinner opened this issue Jun 23, 2023 · 8 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement

Comments

@vstinner
Copy link
Member

vstinner commented Jun 23, 2023

The Python stdlib sys module implements multiple API with simple attributes like sys.path, sys.modules, etc. Converting these APIs to getter and setter functions would be an annoying and expensive migration, a lot of code expect these attributes and access them directly.

It's not possible to execute code before or after a module attribute is set to validate inputs or to update internal states.

Example:

$ python3.12
>>> import sys
>>> sys.modules = 123

>>> import asyncio
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<frozen importlib._bootstrap>", line 1260, in _find_and_load
AttributeError: 'int' object has no attribute 'get'

If sys.modules is set to an invalid type, import stops working and produces an error which makes no sense.

Another problem is that the internals of Python have a copy of sys attributes and so don't respect sys changes. Recently, I discovered that PyImport_AddModule() uses a "copy" of sys.modules (a reference to the dict). Overriding sys.modules has no effect: it still changes the original sys.modules dict. See #105998 (comment) for technical details.

Supporting __setattr__() would allow to update the internal reference to sys.modules (PyInterpreterState.imports.modules).

Adding __delattr__() would help prevent to delete critical sys attributes which makes the code more complicated. For example, code using sys.stderr has to check if the attribute exists and if it's not None. Currently, it's possible to remove any sys attribute, including functions:

$ python3.12
>>> import sys
>>> del sys.excepthook
>>> 1+
sys.excepthook is missing
  File "<stdin>", line 1
    1+
      ^
SyntaxError: invalid syntax

Notice the sys.excepthook is missing error mesage.

In Python 3.7, support for __getattr__() and dir() was added to modules by PEP 562 – Module __getattr__ and __dir__.


This change alone doesn't allow to validate changes of mutable objects like sys.path list. For example, it was proposed to deprecate or disallow adding bytes strings to sys.path. For this, sys.path should use a different type (such as a list subclass): it's not directly related to this issue.

Linked PRs

@vstinner vstinner added the type-bug An unexpected behavior, bug, or error label Jun 23, 2023
@vstinner
Copy link
Member Author

Currently, it's possible to implement this feature by overriding the module in sys.modules with a different class which implements __setattr__(). Proof-of-concept mymodule.py:

# regular module code
attr = 123

def func():
    print("called func()")

# override the module in sys.modules to implement __setattr__()
class FakeModule:
    def __init__(self, ns):
        vars(self).update(ns)

    def __setattr__(self, name, value):
        print("setting %s=%s" % (name, value))
        object.__setattr__(self, name, value)

import sys
sys.modules[__name__] = FakeModule(globals())

Example:

$ python3
>>> import mymodule
>>> mymodule.x = 1
setting x=1

But this approach doesn't work for C extensions (which is what I care about here) ;-)

I found the code (that I updated for Python 3) in an old comp.lang.python discussion (2010) about overriding __setattr__() of a module: https://groups.google.com/g/comp.lang.python/c/2H53WSCXhoM

This feature was also requested recently: https://discuss.python.org/t/extend-pep-562-with-setattr-for-modules/25506

@erlend-aasland
Copy link
Contributor

Feature or bug? ;)

@vstinner vstinner added type-feature A feature request or enhancement and removed type-bug An unexpected behavior, bug, or error labels Jun 23, 2023
@vstinner
Copy link
Member Author

Feature or bug? ;)

Feature request :-)

@erlend-aasland
Copy link
Contributor

Since, module __getattr__ and __dict__ required a (small) PEP (PEP-562), I assume this feature should also get a (small) PEP :)

@skirpichev
Copy link
Member

I assume this feature should also get a (small) PEP :)

I did a draft PEP, with __setattr__ support only. Despite for some support from core devs, there are no sponsors yet.

Let me know if it does make sense to extend this effort.

skirpichev added a commit to skirpichev/cpython that referenced this issue Aug 22, 2023
vstinner added a commit to vstinner/cpython that referenced this issue Aug 22, 2023
* Move Python scripts related to test_module to this new directory:
  good_getattr.py and bad_getattrX.py scripts.
* Move Lib/test/test_module.py to Lib/test/test_module/__init__.py.
vstinner added a commit that referenced this issue Aug 22, 2023
* Move Python scripts related to test_module to this new directory:
  good_getattr.py and bad_getattrX.py scripts.
* Move Lib/test/test_module.py to Lib/test/test_module/__init__.py.
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Aug 22, 2023
* Move Python scripts related to test_module to this new directory:
  good_getattr.py and bad_getattrX.py scripts.
* Move Lib/test/test_module.py to Lib/test/test_module/__init__.py.
(cherry picked from commit adfc118)

Co-authored-by: Victor Stinner <[email protected]>
vstinner added a commit to vstinner/cpython that referenced this issue Aug 22, 2023
* Move Python scripts related to test_module to this new directory:
  good_getattr.py and bad_getattrX.py scripts.
* Move Lib/test/test_module.py to Lib/test/test_module/__init__.py.

(cherry picked from commit adfc118)
skirpichev added a commit to skirpichev/cpython that referenced this issue Aug 22, 2023
vstinner added a commit that referenced this issue Aug 22, 2023
…08304)

gh-106016: Add Lib/test/test_module/ directory (GH-108293)

* Move Python scripts related to test_module to this new directory:
  good_getattr.py and bad_getattrX.py scripts.
* Move Lib/test/test_module.py to Lib/test/test_module/__init__.py.
(cherry picked from commit adfc118)

Co-authored-by: Victor Stinner <[email protected]>
Yhg1s pushed a commit that referenced this issue Aug 22, 2023
)

gh-106016: Add Lib/test/test_module/ directory (#108293)

* Move Python scripts related to test_module to this new directory:
  good_getattr.py and bad_getattrX.py scripts.
* Move Lib/test/test_module.py to Lib/test/test_module/__init__.py.

(cherry picked from commit adfc118)
@vstinner vstinner changed the title Support defining __setattr__() and __delattr__() in a module PEP 726: Support defining __setattr__() and __delattr__() in a module Nov 8, 2023
@joaoe
Copy link

joaoe commented Nov 11, 2023

Hi.

I see there is a PEP for this
https://peps.python.org/pep-0726/

There is a use case I've encountered recently which would require a proper efficient support for setattr and that is the warnings module.
As you well know, it is not thread safe, and that module has a lot of monkeypatch going one replacing the showwarning function. Exactly what is described in that PEP.

Fixing that would require changing how catch_warnings() and filters work, so that no monkeypatching happens. However, that is very non-backwards compatible if 3rd parties are trying to do the same. A way to fix that would be to detect attributes being set in the module and issue an deprecation warning. Later fail with an error. And that requires a module.setattr. However, adding a class for such an important module could perhaps represent other performance or compatibility problems.

#91505

@skirpichev
Copy link
Member

@joaoe, I think this scenario at least partially mentioned in the discussion thread: https://discuss.python.org/t/32640/19. And also in the PEP (the last paragraph of the "Discussion" section), let me know if you feel this should be rephrased somehow. I'm going to submit a SC issue to ask for final decision.

Practical benefits for package or module maintainers are very important (see https://discuss.python.org/t/32640/17) to the fate of PEP. So, I'm suggesting to share your thoughts in the discussion thread.

@iritkatriel iritkatriel added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Nov 29, 2023
@vstinner
Copy link
Member Author

The decision is now in the hands of the Steering Council: python/steering-council#216 For now, I prefer to close this issue. Another issue can be opened once the PEP will be approved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

5 participants