biqqles / dataclassy

A fast and flexible reimplementation of data classes
https://pypi.org/project/dataclassy
Mozilla Public License 2.0
81 stars 9 forks source link

Use of super() in base class raises TypeError #36

Open hjalmarlucius opened 3 years ago

hjalmarlucius commented 3 years ago

First of all thanks for a great tool!

For some reason, this raises TypeError: super(type, obj): obj must be an instance or subtype of type:

from dataclassy import dataclass

@dataclass
class Dummy:
    def __setattr__(self, name, value):
        # do some checks
        super().__setattr__(name, value)

d = Dummy()
d.x = None
biqqles commented 3 years ago

This is very curious, I will look into it.

biqqles commented 3 years ago

Ah, my guess is that super() doesn't like that what Dummy is has changed between the time __setattr__ was defined and the time it is called. This is because dataclassy recreates the class when converting it into a data class.

This may not be what you want, depending on how you want to subclass Dummy, but you could consider:

@dataclass
class Dummy:
    def __setattr__(self, name, value):
        # do some checks
        object.__setattr__(self, name, value)

d = Dummy()
d.x = None

which works fine.

hjalmarlucius commented 3 years ago

ok thanks!

biqqles commented 3 years ago

Did that solve your issue fully? I still want to get to the bottom of this so I will keep this open regardless.

hjalmarlucius commented 3 years ago

Not really, I'm building a lib heavy on mixins so need super to ensure all parents get a chance to do their job if they have anything. My reason for dataclassy is that non-savvy users are supposed to add custom datastructures on top of these mixins - so this interface should be clean and simple to explain (which this lib is).

Since then, I've better understood how super with explicit arguments work so it might be solvable that way. For now I'm doing it manually.

Thx for the followup! Brief feedback on the lib:

The reason why I went with dataclassy were: a) short codebase so I could read and understand the mechanics b) slots c) more compact interface than attrs

Wishlist:

biqqles commented 3 years ago

Thanks for your detailed reply.

so I could read and understand the mechanics

This was nice to read! It's why I advertise dataclassy's small size (25% of the LOC of dataclasses, excluding comments). I place great importance on being able to quickly find your way around and understand how something is done.

Without them, I could not find a way to make something frozen if you e.g. first need to cast it to some type.

Yes, this is a problem with __post_init__ and frozen in general. The workaround is using object.__setattr__ explicitly. There's no easy solution though I'd like to find one. Personally, I stopped using frozen=True altogether when I noticed its ridiculous performance overhead (thanks to the generated __init__ having to use object.__setattr__ itself).

Factory option for non-standard constructors (or simply call any zero-arg lambda func).

Everyone is asking for this so it will be added, hopefully soon!

biqqles commented 3 years ago

See https://github.com/biqqles/dataclassy/issues/34#issuecomment-824197151 for progress on the last wish.

biqqles commented 3 years ago

By the way, I really like your converter idea too.

Better workaround for this issue:

from dataclassy import dataclass

@dataclass
class DummyBase:
    pass

class Dummy(DummyBase):
    def __setattr__(self, name, value):
        # do some checks
        super().__setattr__(name, value)

d = Dummy()
d.x = None

This works because the problem is specific to super() usage in the first class @dataclass is used on.

biqqles commented 3 years ago

Interesting, I was searching for solutions to this and found that attrs had the same issue https://github.com/python-attrs/attrs/issues/102. I didn't even know it supported slots! The fix is messy, but I expected to have to do something with the function's cell https://github.com/python-attrs/attrs/pull/226/commits/cd0cc7fd9e1233d9c9f4fd736227a0c8be26ba1e.

yesthesoup commented 1 year ago

I've run into this as well with dataclassy 1.0.1 and python 3.10, making use of __init_subclass__ (link to 3.10 docs for this method), which can be reproduced by tweaking the example in its PEP

>>> from dataclassy import dataclass
>>> @dataclass
... class QuestBase:
...    def __init_subclass__(cls, **kwargs):
...        super().__init_subclass__(**kwargs)

>>> class Quest(QuestBase):
...    pass
...
Traceback (most recent call last):
  File "[...]/.pyenv/versions/3.10.7/lib/python3.10/code.py", line 90, in runcode
    exec(code, self.locals)
  File "<console>", line 1, in <module>
  File "[...]/venv/lib/python3.10/site-packages/dataclassy/dataclass.py", line 140, in __new__
    return super().__new__(mcs, name, bases, dict_)
  File "<console>", line 4, in __init_subclass__
TypeError: super(type, obj): obj must be an instance or subtype of type

If you reproduce the original example exactly, a different error but at the same line:

>>> from dataclassy import dataclass
>>> @dataclass
... class QuestBase:
...     def __init_subclass__(cls, swallow, **kwargs):
...             cls.swallow = swallow
...             super().__init_subclass__(**kwargs)
...
>>> class Quest(QuestBase, swallow="african"):
...     pass
...
Traceback (most recent call last):
  File "[...]/.pyenv/versions/3.10.7/lib/python3.10/code.py", line 90, in runcode
    exec(code, self.locals)
  File "<console>", line 1, in <module>
  File "[...]/venv/lib/python3.10/site-packages/dataclassy/dataclass.py", line 140, in __new__
    return super().__new__(mcs, name, bases, dict_)
TypeError: QuestBase.__init_subclass__() missing 1 required positional argument: 'swallow'

I can make a separate new issue instead if this doesn't belong here.

ETA: the suggestion of an empty DummyBase class does work.

biqqles commented 1 year ago

Your comment is in the right place I think.

Disappointed this hasn't been fixed in newer CPython versions, assuming it can be. The reason I never worked around this was simply that the fix is hideous and involves rewriting the closure cell with ctypes (https://github.com/python-attrs/attrs/commit/cd0cc7fd9e1233d9c9f4fd736227a0c8be26ba1e). It didn't seem worth it in a deliberately "lightweight" library for a case that is pretty rare I think.

Not tried this but presumably, in lieu of the dummy class, you could just change the call to object.__init_subclass__?

yesthesoup commented 1 year ago

Gotcha - yes I would say not worth rewriting all that for this library.

Your suggestion of object.__init_subclass__ does work as well, so either that or the DummyBase is fine as a workaround to me.

biqqles commented 1 year ago

Maybe a note in the readme would be good, as you're the second person at least to run into this.

mara004 commented 4 weeks ago

This is because dataclassy recreates the class when converting it into a data class.

IMO cloning is bad practice. This should be avoidable in the first place by using a metaclass rather than decorator interface.