• cattrs version: 1.1.1
• Python version: 3.7
• Operating System: Mac

Description

The old converter supports attrs classes which inherit from each other because it doesn't use MRO-based singledispatch per type to dispatch its attrs classes - therefore, when a subclass B is encountered, it is dynamically structured based on the actual type and the attributes available in the dict.

However, the new GenConverter creates and registers a single-dispatched function as soon as it sees a new type. And functools.singledispatch seems to do some MRO stuff and identifies any subclass as "being" its parent class, therefore skipping the code generation and registration process that cattrs would otherwise do for any new subclass.

What I Did

from cattr.converters import GenConverter
import attr


@attr.s(auto_attribs=True)
class A:
    i: int


@attr.s(auto_attribs=True)
class B(A):
    j: int


def test_inheritance():
    converter = GenConverter()

    # succeeds
    assert A(1) == converter.structure(dict(i=1), A)

    # fails
    assert B(1, 2) == converter.structure(dict(i=1, j=2), B)

the failure is

E       AssertionError: assert B(i=1, j=2) == A(i=1)
E        +  where B(i=1, j=2) = B(1, 2)

essentially, cattrs/singledispatch chooses A and then runs the A structuring code, rather than generating a new method.

How to fix this? I know singledispatch should be fairly well optimized, so it would be un-fun to drop it entirely. But since this is a breaking change from the old converter to the new converter, I feel like we should try to do something.

I wonder if it might be just as well to use a strict type map looking in the multistrategy dispatch before we hit the singledispatch function. Essentially, if there's an exact match, use that before even trying singledispatch. If there isn't, and the type is an attrs class, skip singledispatch so that the class is "caught" by the fallback function dispatch.

If you're willing to accept this approach, I found a patch that works nicely. I'll probably put up a PR in the next couple of days.