Skip to content
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

AttributeError on with super() #129466

Open
edwardwkrohne opened this issue Jan 30, 2025 · 11 comments
Open

AttributeError on with super() #129466

edwardwkrohne opened this issue Jan 30, 2025 · 11 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement

Comments

@edwardwkrohne
Copy link

edwardwkrohne commented Jan 30, 2025

Bug report

Bug description:

Writing with super() raises an AttributeError. I ran into this implementing context managers inspired by contextlib.contextmnager but which had extra functionality. I can see no reason why with super() should be disallowed, and in fact, super().__enter__() and super().__exit__() work just fine.

Attempting to use an ExitStack as a work around was unsuccessful.

import sys
from contextlib import ExitStack

class Base:
    def __enter__(self):
        print("Entering base context manager")

    def __exit__(self, type, value, traceback):
        print("Exiting base context manager")

def __derived_enter__(self):
    self.context = self.contextlib_style_context()
    next(self.context)
    return self.context

def __derived_exit__(self, type, value, traceback):
    try:
        next(self.context)
    except StopIteration:
        pass

class DerivedUsingEnterExit(Base):
    def contextlib_style_context(self):
        print("Entering contextlib style context manager")
        super().__enter__()
        try:
            yield
        finally:
            super().__exit__(*sys.exc_info())

        print("Exiting contextlib style context manager")

    __enter__ = __derived_enter__
    __exit__ = __derived_exit__

class DerivedUsingWith(Base):
    def contextlib_style_context(self):
        print("Entering contextlib style context manager")
        with super():
            yield

        print("Exiting contextlib style context manager")

    __enter__ = __derived_enter__
    __exit__ = __derived_exit__

class DerivedUsingExitStack(Base):
    def contextlib_style_context(self):
        print("Entering contextlib style context manager")

        with ExitStack() as stack:
            stack.enter_context(super())
            yield

        print("Exiting contextlib style context manager")

    __enter__ = __derived_enter__
    __exit__ = __derived_exit__

# Runs successfully
with DerivedUsingEnterExit() as d:
    pass

# Raises AttributeError
with DerivedUsingWith() as d:
    pass

# Raises AttributeError
with DerivedUsingExitStack() as d:
    pass

CPython versions tested on:

3.13, 3.12

Operating systems tested on:

Windows

@edwardwkrohne edwardwkrohne added the type-bug An unexpected behavior, bug, or error label Jan 30, 2025
@encukou encukou added type-feature A feature request or enhancement interpreter-core (Objects, Python, Grammar, and Parser dirs) and removed type-bug An unexpected behavior, bug, or error labels Jan 30, 2025
@JeffersGlass
Copy link
Contributor

JeffersGlass commented Jan 30, 2025

The reason that it's currently an error (a TypeError, it seems?) is do to how the enter_context function works: it looks up the type of the object it's passed, then attempts to retrieve the __enter__ and __exit__ methods of that type. But super() returns a super object, which does not itself have the __enter__ and __exit__ methods.

cpython/Lib/contextlib.py

Lines 515 to 529 in 4e47e05

def enter_context(self, cm):
"""Enters the supplied context manager.
If successful, also pushes its __exit__ method as a callback and
returns the result of the __enter__ method.
"""
# We look up the special methods on the type to match the with
# statement.
cls = type(cm)
try:
_enter = cls.__enter__
_exit = cls.__exit__
except AttributeError:
raise TypeError(f"'{cls.__module__}.{cls.__qualname__}' object does "
f"not support the context manager protocol") from None

Perhaps the __enter__ and __exit__ methods could be special-cased within the super object to lookup on the wrapped class? Or super objects could be special-cased in contextlib lookup these methods on the correct class?

I'm not convinced that's the best way forward... but I'd be happy to take a swing at a solution.

Edit: this is the case for 3.12 and 3.13; for 3.14 after alpha one, see the next post.

@JeffersGlass
Copy link
Contributor

JeffersGlass commented Jan 30, 2025

Hmmm the error actually comes from a different place in 3.14. Specifically, from the bytecode implementation of LOAD_SPECIAL (currently on main).

cpython/Python/bytecodes.c

Lines 3284 to 3296 in 4e47e05

inst(LOAD_SPECIAL, (owner -- attr, self_or_null)) {
assert(oparg <= SPECIAL_MAX);
PyObject *owner_o = PyStackRef_AsPyObjectSteal(owner);
PyObject *name = _Py_SpecialMethods[oparg].name;
PyObject *self_or_null_o;
PyObject *attr_o = _PyObject_LookupSpecialMethod(owner_o, name, &self_or_null_o);
if (attr_o == NULL) {
if (!_PyErr_Occurred(tstate)) {
_PyErr_Format(tstate, PyExc_TypeError,
_Py_SpecialMethods[oparg].error,
Py_TYPE(owner_o)->tp_name);
}
ERROR_IF(true, error);

This is based on the specific verbiage of the error messages I'm seeing about missing methods:

cpython/Python/ceval.c

Lines 377 to 381 in 4e47e05

const _Py_SpecialMethod _Py_SpecialMethods[] = {
[SPECIAL___ENTER__] = {
.name = &_Py_ID(__enter__),
.error = "'%.200s' object does not support the "
"context manager protocol (missed __enter__ method)",

So again, some special-casing could occur there.

@bswck
Copy link
Contributor

bswck commented Jan 30, 2025

+1, the special-cased behavior would be very intuitive and succinct

@edwardwkrohne
Copy link
Author

edwardwkrohne commented Jan 30, 2025

I'm trying to understand LOAD_SPECIAL here, I've never gotten into the opcodes. The picture I'm getting is that LOAD_SPECIAL is used for dunders like __exit__ when used in whatever their special capacities are, and super doesn't have explicit support for LOAD_SPECIAL, so super can't be used in things like with, for, next, etc. Is that correct? And I suppose LOAD_SPECIAL is not used when dunders are accessed explicitly, which is why super().__enter__() works?

I'd be in favor of adding that support to super, or otherwise special casing.

Those changes to super by themselves wouldn't allow stack.enter_context(super()) because super is not a class that has the right methods; so ExitStack would also require explicit special casing in the standard library. I think the special case would be justified there too, since super really is a unique type of object that may need special handling, and if you can with something, you ought to be able to stack.enter_context it.

I could submit an ExitStack patch easily enough, but since I'm not familiar with the opcodes I doubt I could change super itself myself without it becoming a massive project.

@picnixz
Copy link
Member

picnixz commented Jan 31, 2025

The docs say:

Accordingly, super() is undefined for implicit lookups using statements or operators such as super()[name].

The use of with super() falls under a statement usage (the __enter__ method is being looked up) hence it's not possible to use it. AFAICT, supporting with super() should mean supporting super()[...] for completeness.

So, strictly speaking, the behaviour is as expected although burried in the docs. Changing this may require a PEP but we may start by trying to provide a recipe for this (writing enter and exit calls explicitly may not be the first thing users may think of so it's better if we can write that recipe down).

OTOH, we may special-case the enter/exit special methods as well as their asynchronous counterparts which suffer from the same issue I think (I don't think async with super() would work and I also wonder whether other asynchronous special methods have that shortcoming or not).

@JeffersGlass
Copy link
Contributor

@picnixz Thank you for pointing that out in the docs - I'd agree with you that this is behaving as described, though I wonder if we can be even clearer there - perhaps adding the explicit example of context managers to the docs as in:

Accordingly, super() is undefined for implicit lookups using statements or operators such as super()[name] or with super()


You're right that the error is raised with async context managers:

import asyncio

class Base:
    def __aenter__(self):
        print("Entering Base")

    def __aexit__(self):
        print("Exiting Base")

class Derived(Base):
    async def foo(self):
        async with super():
            print("Inside context manager")

asyncio.run(Derived().foo())
TypeError: 'super' object does not support the asynchronous context manager protocol (missed __aexit__ method)

__enter__, __exit__, __aenter__, and __aexit__ are the methods special-cased in ceval.c (in _Py_SpecialMethods[]).

@picnixz
Copy link
Member

picnixz commented Feb 1, 2025

We had an opened issue that aimed to improve the error messages for those cases. Since there was no progression since then, I will have a look at this in two weeks or so. In the meantime I'm wondering whether we can decide first whether we want to support this kind of feature or not.

super() is meant to be a proxy but it's not a perfect proxy (it's not just a "we pick the correct method from the MRO and dispatch") as the docs say. Since I'm not an expert in the super() specs, I'll ask some typing experts @JelleZijlstra and @carljm, as well as some other core devs @serhiy-storchaka (for pickle possible implications) and @rhettinger (who wrote a guide for using super()).

@carljm
Copy link
Member

carljm commented Feb 1, 2025

I don't think this is really a typing question, just a language question. If we add the behavior to super that it supports the context manager protocol by looking up __enter__ and __exit__ from the MRO, type checkers will adapt to that change.

I don't have strong feelings either way about the change. My main hesitance would be that it's not clear where we should stop; should super objects also support __getitem__? __eq__ and comparison methods? Many other special methods? It may be better to keep the clear line that super just supports attribute access, which is the only thing it needs to support in order to fulfill its core function.

The implementation might also get a little weird, in that I think the __enter__ and __exit__ methods of the super type would have to copy/emulate the normal runtime error messages that occur when an object doesn't support the context manager protocol, if it can't find __enter__ and __exit__ in the MRO.

There could also be strange/surprising behaviors if super finds __enter__ and __exit__ in different places in the MRO? Though this implies some type is defining one but not the other, which would be unusual.

I think it would be pretty easy to write a small library class that could wrap a super object (or, really, any object with __enter__ and __exit__ attributes) and implement the context manager protocol. It would do the same thing that this issue proposes super do: implement __enter__ and __exit__ methods that proxy to those methods on the wrapped object. Then the code above could just be changed to with ProxyContext(super()):. It seems to me that's not too onerous for an uncommon need, and may be a better solution than building this directly into super.

@picnixz
Copy link
Member

picnixz commented Feb 1, 2025

Ah yes Carl, sorry if I used "typing" here instead of "language". I forgot the word for a moment and went for the closest one I could remember :')

There could also be strange/surprising behaviors if super finds enter and exit in different places in the MRO? Though this implies some type is defining one but not the other, which would be unusual.

This is what happens if we have mixin classes where a mixin implements __enter__ and the other implements __exit__ combined with a diamond inheritance. But this is perhaps not the best real-world example..

Then the code above could just be changed to with ProxyContext(super()):

How about adding this to contextlib recipes (not necessarily implement it on our side). Orthogonally, we can also list a few more cases where <something>super()<something-else> doesn't work (currently we only list the super()[index] but we can also list with super()).

@carljm
Copy link
Member

carljm commented Feb 1, 2025

How about adding this to contextlib recipes (not necessarily implement it on our side). Orthogonally, we can also list a few more cases where <something>super()<something-else> doesn't work (currently we only list the super()[index] but we can also list with super()).

Sure, those both seem like potentially reasonable things to do. The contextlib recipe would just need to be evaluated for how common this need is, and whether there's concern about bloating the contextlib recipes with too many very niche recipes.

@picnixz
Copy link
Member

picnixz commented Feb 1, 2025

Just to be clear, but by recipes, I only meant docs recipes. I don't think we have a page for them yet though (but we should perhaps have a cookbook for more modules in general).

I'll start a dpo thread next week on this matter (namely whether we should have more cookbooks for modules and how niche we should go)

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

6 participants