[issue46382] dataclass(slots=True) does not account for slots in base classes

[Eric V. Smith]

Eric V. Smith  added the comment:

Thanks for all of your work, @ariebovenberg!

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46382] dataclass(slots=True) does not account for slots in base classes

[Eric V. Smith]

Eric V. Smith  added the comment:


New changeset 82e9b0bb0ac44d4942b9e01b2cdd2ca85c17e563 by Arie Bovenberg in 
branch 'main':
bpo-46382 dataclass(slots=True) now takes inherited slots into account 
(GH-31980)
https://github.com/python/cpython/commit/82e9b0bb0ac44d4942b9e01b2cdd2ca85c17e563


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46382] dataclass(slots=True) does not account for slots in base classes

[Arie Bovenberg]

Change by Arie Bovenberg :


--
keywords: +patch
pull_requests: +30070
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/31980

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46382] dataclass(slots=True) does not account for slots in base classes

[Eric V. Smith]

Eric V. Smith  added the comment:

I thought there was an existing issue that covered this, but now I can't find 
it.

I'd prefer #2, create a separate issue.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46382] dataclass(slots=True) does not account for slots in base classes

[Arie Bovenberg]

Arie Bovenberg  added the comment:

@eric.smith awesome! 

What would you like to do about the __weakref__ slot?

1. take no action
2. create a separate ticket to discuss further
3. implement as done in attrs, now that we're changing __slots__ dataclass 
behavior anyway

I wouldn't recommend (1), because sooner or later this feature request will pop 
up. It seems reasonable to support it.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46382] dataclass(slots=True) does not account for slots in base classes

[Eric V. Smith]

Eric V. Smith  added the comment:

I don't have a problem saying that for a class to be used as a base class for a 
dataclass, its __slots__ must not be an iterator that's been exhausted. That 
doesn't seem like a very onerous requirement.

I'm also not concerned about people using __slots__ to iterate over the fields, 
but I agree that a documentation note couldn't hurt.

@ariebovenberg: go ahead and submit your PR. Thanks!

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46382] dataclass(slots=True) does not account for slots in base classes

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

Sorry, the previous message was written via a phone because of blackout, so the 
formatting is a tine bit messy.

__slotnames__ is set in copyreg._slotnames(). If you want to keep a list of all 
slots you should save it in __slotnames__, either using copyreg._slotnames() 
directly or by duplicating its code.

But as for the original issue, I do not think that the current behavior is 
incorrect. At least it is consistent with the behavior of regular classes.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46382] dataclass(slots=True) does not account for slots in base classes

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

>
> It is not set automatically. It is set in object.__reduce__ by calling
> some helper function from the copyreg module.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46382] dataclass(slots=True) does not account for slots in base classes

[Eric V. Smith]

Eric V. Smith  added the comment:

Serhiy: Could you point to some documentation on __slotnames__? I see a few 
references in the code to it, but it's not set on simple test class.

>>> class A:
... __slots__=('a',)
...
>>> A.__slotnames__
Traceback (most recent call last):
  File "", line 1, in 
AttributeError: type object 'A' has no attribute '__slotnames__'. Did you mean: 
'__slots__'?
>>>

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46382] dataclass(slots=True) does not account for slots in base classes

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

__slotnames__ is used for storing all slot names.

New object.__getstate__() proposed in issue26579 may have some relation to this 
discussion.

--
nosy: +serhiy.storchaka

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46382] dataclass(slots=True) does not account for slots in base classes

[Arie Bovenberg]

Arie Bovenberg  added the comment:

@eric.smith did you give this some thought? Would we want to imitate the attrs 
behavior regarding __weafref__?

It'd be nice if I can submit a PR to be included in 3.11

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46382] dataclass(slots=True) does not account for slots in base classes

[Arie Bovenberg]

Arie Bovenberg  added the comment:

@hynek interesting! 

The discussion in https://github.com/python-attrs/attrs/pull/420 on the weakref 
slot is very interesting as well.

Considering __weakref__ is something we don't want to make impossible in 
dataclasses, @eric.smith what would be your preferred solution?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46382] dataclass(slots=True) does not account for slots in base classes

[Hynek Schlawack]

Hynek Schlawack  added the comment:

>>> @attrs.define
... class C(Base):
...   a: int
...   b: int
...
>>> C.__slots__
('b', '__weakref__')

We've got a test specifically for this use case: 
https://github.com/python-attrs/attrs/blob/5f36ba9b89d4d196f80147d4f2961fb2f97ae2e5/tests/test_slots.py#L309-L334

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46382] dataclass(slots=True) does not account for slots in base classes

[Alex Waygood]

Change by Alex Waygood :


--
nosy: +hynek

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46382] dataclass(slots=True) does not account for slots in base classes

[Eric V. Smith]

Eric V. Smith  added the comment:

It would also be interesting to see what attrs does in this case.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46382] dataclass(slots=True) does not account for slots in base classes

[Arie Bovenberg]

Arie Bovenberg  added the comment:

Spencer is correct.

The documentation even adds: "This renders the meaning of the program 
undefined."

It's clear it doesn't break anything users would often encounter (we would have 
heard about it), but it's still undefined behavior.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46382] dataclass(slots=True) does not account for slots in base classes

[Alex Waygood]

Change by Alex Waygood :


--
nosy: +AlexWaygood

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46382] dataclass(slots=True) does not account for slots in base classes

[Spencer Brown]

Spencer Brown  added the comment:

Both will function, but class B will add its slots after A's, causing there to 
be an extra unused slot in the object that you can only access by directly 
using the A.a descriptor. So all slotted inheriting dataclasses cause the 
object to use more memory than necessary.

--
nosy: +Spencer Brown

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46382] dataclass(slots=True) does not account for slots in base classes

[Nikita Sobolev]

Nikita Sobolev  added the comment:

Arie, can you please explain what is the technical difference between these two 
cases:

```python
class A:
__slots__ = ('a', )
# fields

class B(A):
__slots__ = ('a', 'b')
# fields
```

And:

```python
class C:
__slots__ = ('a', )
# fields

class D(C):
__slots__ = ('b', )
# fields
```

?

--
nosy: +sobolevn

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46382] dataclass(slots=True) does not account for slots in base classes

[Arie Bovenberg]

Arie Bovenberg  added the comment:

There are already 2 complexities I can think of:

1. This behavior may break some people's code, if they use __slots__ to iterate 
over
   the fields of a dataclass. Solution: explicitly mention in the docs that
   not every field may get a slot on the new class. Advise them to use
   `fields()` to iterate over the fields.
2. It's technically allowed for __slots__ to be an iterator (which will then be 
   exhausted at class creation). Finding the __slots__ of such a class
   may require more elaborate introspection.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46382] dataclass(slots=True) does not account for slots in base classes

[Eric V. Smith]

Eric V. Smith  added the comment:

I'll have to do some more research. But your analysis looks correct to me, so 
far.

--
assignee:  -> eric.smith
nosy: +eric.smith

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46382] dataclass(slots=True) does not account for slots in base classes

[Arie Bovenberg]

New submission from Arie Bovenberg :

@dataclass(slots=True) adds slots to dataclasses. It adds a slot per field. 
However, it doesn't account for slots already present in base classes:

>>> class Base:
... __slots__ = ('a', )
...
>>> @dataclass(slots=True)
... class Foo(Base):
... a: int
... b: float
...
>>> Foo.__slots__
('a', 'b')  # should be: ('b', )


The __slots__ documentation says:

If a class defines a slot also defined in a base class, the instance 
variable 
defined by the base class slot is inaccessible (except by retrieving its 
descriptor 
directly from the base class). This renders the meaning of the program 
undefined. 
In the future, a check may be added to prevent this.

Solution: don't add slots which are already defined in any base classes:

>>> @dataclass
... class Bla(Base):
... __slots__ = ('b', )
... a: int
... b: float
...
>>> Bla(4, 5.65)
Bla(a=4, b=5.65)

If you agree, I'd like to submit a PR to fix this. I already have a prototype 
working.

--
components: Library (Lib)
messages: 410591
nosy: ariebovenberg
priority: normal
severity: normal
status: open
title: dataclass(slots=True) does not account for slots in base classes
type: behavior
versions: Python 3.10, Python 3.11

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com