biqqles / dataclassy

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

Bug when mixing metaclasses w/ `dataclassy` (Python 3.6 only) #52

Open fubuloubu opened 3 years ago

fubuloubu commented 3 years ago

I found a bug when I attempt to mix metaclasses w/ dataclassy. Here is what I was doing:

@abstractdataclass
class ConverterAPI(Generic[ABIType]):
    ...

    @abstractmethod
    def convert(self, value: str) -> ABIType:
        ...

Generic has nothing to do with dataclassy' mechanics, but I could see how the metaclass might interfere.

This is the error it raises in Python 3.6 (from my CI):

...
    class ConverterAPI(Generic[ABIType]):
/opt/hostedtoolcache/Python/3.6.13/x64/lib/python3.6/site-packages/dataclassy/decorator.py:40: in dataclass
    return apply_metaclass(cls)
/opt/hostedtoolcache/Python/3.6.13/x64/lib/python3.6/site-packages/dataclassy/decorator.py:35: in apply_metaclass
    return metaclass(to_class.__name__, to_class.__bases__, dict_, **options)
/opt/hostedtoolcache/Python/3.6.13/x64/lib/python3.6/site-packages/dataclassy/dataclass.py:137: in __new__
    return super().__new__(mcs, name, bases, dict_)
/opt/hostedtoolcache/Python/3.6.13/x64/lib/python3.6/abc.py:133: in __new__
    cls = super().__new__(mcls, name, bases, namespace, **kwargs)
E   TypeError: metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases

For reference, here is the code that defines the abstractdataclass decorator:

from abc import ABCMeta, abstractmethod
from functools import partial
from dataclassy import dataclass
from dataclassy.dataclass import DataClassMeta

class AbstractDataClassMeta(DataClassMeta, ABCMeta):
    pass

abstractdataclass = partial(dataclass, kwargs=True, meta=AbstractDataClassMeta)

It works perfectly fine with Python 3.7+, I think related to this: https://stackoverflow.com/questions/11276037/resolving-metaclass-conflicts

biqqles commented 3 years ago

As I see it there are three possibilities that explain this (ranked in order of my guesses at likelihood):

1) The implementation of typing.Generic changed between 3.6 and 3.7. I think this is most likely because the typing module underwent a lot of changes between these versions IIRC 2) The implementation of abc.ABCMeta changed 3) Python became somehow "better" at resolving metaclass mixing

I will take some time later to try to prove or disprove these theories, then hopefully fix the problem if it's with dataclassy, or find a workaround.

biqqles commented 3 years ago

MRE:

from typing import Generic, TypeVar
from dataclassy import dataclass

X = TypeVar('X')

@dataclass
class T(Generic[X]):
    ...
biqqles commented 3 years ago

Diff: https://github.com/python/cpython/compare/v3.6.14...v3.7.10 Unfortunately I can't find a way to link to the single file typing.py.

fubuloubu commented 3 years ago

Diff: python/cpython@v3.6.14...v3.7.10 Unfortunately I can't find a way to link to the single file typing.py.

These seems relevant (in typing.py)

image

image

So, seems like in Python <=3.6 only you'd need to integrate GenericMeta into dataclassy's meta to support Generics

fubuloubu commented 3 years ago

So, seems like in Python <=3.6 only you'd need to integrate GenericMeta into dataclassy's meta to support Generics

Or... I can just drop 3.6 support from my library :sweat: (which I've been meaning to anyways)

biqqles commented 3 years ago

Great find. The 3.7 release notes are a little more blunt:

the generic types can be used without metaclass conflicts

This kind of makes me want to say it's a typing problem, but I'm not against a fix in dataclassy if it's easy.

Anyway, this made me curious what the same paragraph looked like in 3.6. There I read:

The metaclass used by Generic is a subclass of abc.ABCMeta.

This made me try implementing class AbstractDataClassMeta(DataClassMeta, GenericMeta): as a sneaky workaround. It seems like it should maybe work but instead of (or before, quite possibly) the conflict TypeError you get TypeError: Cannot inherit from plain Generic when actually using the decorator.

Edit: trying your suggestion of basing DataClassMeta on GenericMeta causes the same error.

fubuloubu commented 3 years ago

@biqqles thanks for looking into it, I've decided to just deprecate support for Python 3.6 in my framework. IPython was also giving me grief, so it made sense to do.

mara004 commented 3 weeks ago

In general, I was under the impression that such situations could be handled on the caller side by merging the separate (conflicting) metaclasses into one via inheritance, similar to how the above AbstractDataClassMeta does? Reading between the lines, it seems like this is, for some reason, not an option with Generic? Or is it? (Sorry if I'm missing something obvious here, I never was much into typing...)