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

Frozen dataclasses with slots raise TypeError #90055

Closed
treyhunner opened this issue Nov 25, 2021 · 8 comments
Closed

Frozen dataclasses with slots raise TypeError #90055

treyhunner opened this issue Nov 25, 2021 · 8 comments
Assignees
Labels
3.10 only security fixes 3.11 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@treyhunner
Copy link
Member

BPO 45897
Nosy @ericvsmith, @treyhunner, @AlexWaygood
PRs
  • bpo-45897: Fix frozen-slotted dataclass bug #29895
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/ericvsmith'
    closed_at = None
    created_at = <Date 2021-11-25.01:53:51.202>
    labels = ['type-bug', 'library', '3.10', '3.11']
    title = 'Frozen dataclasses with slots raise TypeError'
    updated_at = <Date 2021-12-02.17:20:40.230>
    user = 'https://github.com/treyhunner'

    bugs.python.org fields:

    activity = <Date 2021-12-02.17:20:40.230>
    actor = 'AlexWaygood'
    assignee = 'eric.smith'
    closed = False
    closed_date = None
    closer = None
    components = ['Library (Lib)']
    creation = <Date 2021-11-25.01:53:51.202>
    creator = 'trey'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 45897
    keywords = ['patch']
    message_count = 4.0
    messages = ['406973', '406974', '406995', '407540']
    nosy_count = 3.0
    nosy_names = ['eric.smith', 'trey', 'AlexWaygood']
    pr_nums = ['29895']
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue45897'
    versions = ['Python 3.10', 'Python 3.11']

    @treyhunner
    Copy link
    Member Author

    When making a dataclass with slots=True and frozen=True, assigning to an invalid attribute raises a TypeError rather than a FrozenInstanceError:

    >>> from dataclasses import dataclass
    >>> @dataclass(frozen=True, slots=True)
    ... class Vector:
    ...     x: float
    ...     y: float
    ...     z: float
    ...
    >>> v = Vector(1, 2, 3)
    >>> v.a = 4
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "<string>", line 5, in __setattr__
    TypeError: super(type, obj): obj must be an instance or subtype of type

    @treyhunner treyhunner added 3.10 only security fixes type-bug An unexpected behavior, bug, or error labels Nov 25, 2021
    @AlexWaygood
    Copy link
    Member

    This looks to be due to the fact that slots=True leads to the creation of an entirely new class (see line 1102), meaning that in the super(cls, self) calls in lines 611 and 618 (in the _frozen_get_del_attr function, responsible for generating __setattr__ and __delattr__ methods), self is no longer an instance of cls.

    I believe this can be fixed by tweaking _frozen_get_del_attr so that cls in the generated __setattr__ and __delattr__ methods is dynamically computed (cls = type(self)), rather than read from a closure, as is currently the case.

    @AlexWaygood AlexWaygood added stdlib Python modules in the Lib dir labels Nov 25, 2021
    @ericvsmith ericvsmith added 3.11 only security fixes labels Nov 25, 2021
    @ericvsmith
    Copy link
    Member

    I think the error should be AttributeError, which is what you'd get if the class weren't frozen.

    @AlexWaygood
    Copy link
    Member

    You get the same error if you subclass a frozen dataclass, then try to set an attribute that is not one of the superclass's __slots__:

    >>> @dataclass(slots=True, frozen=True)
    ... class Point:
    ...     x: int
    ...     y: int
    ... 
    ...     
    >>> class Subclass(Point): pass
    ... 
    >>> s = Subclass(1, 2)
    >>> s.z = 5
    Traceback (most recent call last):
      File "<pyshell#15>", line 1, in <module>
        s.z = 5
      File "<string>", line 7, in __setattr__
    TypeError: super(type, obj): obj must be an instance or subtype of type
    

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @MojoVampire
    Copy link
    Contributor

    MojoVampire commented May 3, 2022

    I believe this can be fixed by tweaking _frozen_get_del_attr so that cls in the generated __setattr__ and __delattr__ methods is dynamically computed (cls = type(self)), rather than read from a closure, as is currently the case.

    It looks like this is not the approach you took, but to be clear, using type(self) for the cls argument to super() does not work. It seems to work when you just have the one class, then becomes an infinite recursion error when you subclass the class (an empty stub subclass is all you need) and try to call the method in question on an instance of the subclass. It needs to be read from the closure because the cls in question must be a definition-time association (the method itself is inherently linked to the class it was defined in); at runtime, there's no other source that will tell you where you were defined (it's why Python hides a __class__ closure scope variable in every method that references __class__ or super, just in case).

    @AlexWaygood
    Copy link
    Member

    I believe this can be fixed by tweaking _frozen_get_del_attr so that cls in the generated __setattr__ and __delattr__ methods is dynamically computed (cls = type(self)), rather than read from a closure, as is currently the case.

    It looks like this is not the approach you took, but to be clear, using type(self) for the cls argument to super() does not work. It seems to work when you just have the one class, then becomes an infinite recursion error when you subclass the class (an empty stub subclass is all you need) and try to call the method in question on an instance of the subclass.

    Yes, this is why this is not the approach I took :) thanks for updating the issue though! I forgot that I'd left that comment

    @ericvsmith
    Copy link
    Member

    Okay, I'm finally getting a chance to look at this. A few things:

    • I don't think we can change this in a minor release (3.10 or 3.11, at this point).
    • Is this even worth fixing? If the code had always raised AttributeError, I would be happy with it. But is the cost of disruption worth it? I realize that very few people probably actually catching this exception, and the value here is at the REPL to give a better error message. But still, there's probably more than zero people this would affect.

    @ericvsmith
    Copy link
    Member

    I'm going to close this. If someone really feels strongly that the exception needs to change, please open a discussion on discuss.python.org. We can re-open this issue if a discussion there produces some consensus.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.10 only security fixes 3.11 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants