configure_tagged_union strategy does not support diamond inheritance

[danarmak]

import cattrs
from attrs import frozen
from cattrs import strategies

@frozen
class Base: pass
@frozen
class Mid1(Base): pass
@frozen
class Mid2(Base): pass
@frozen
class Sub(Mid1, Mid2): pass

converter = cattrs.Converter()
strategies.include_subclasses(Base, converter, union_strategy=strategies.configure_tagged_union)

Raises the error:

Traceback (most recent call last):
  File "/home/danarmak/sb/aoa/agentune-analyze/agentune/playground.py", line 15, in <module>
    strategies.include_subclasses(Base, converter, union_strategy=strategies.configure_tagged_union)
  File "/home/danarmak/.cache/pypoetry/virtualenvs/agentune-analyze-y3VXCvbo-py3.12/lib/python3.12/site-packages/cattrs/strategies/_subclasses.py", line 84, in include_subclasses
    _include_subclasses_with_union_strategy(
  File "/home/danarmak/.cache/pypoetry/virtualenvs/agentune-analyze-y3VXCvbo-py3.12/lib/python3.12/site-packages/cattrs/strategies/_subclasses.py", line 237, in _include_subclasses_with_union_strategy
    union_strategy(u, converter)
  File "/home/danarmak/.cache/pypoetry/virtualenvs/agentune-analyze-y3VXCvbo-py3.12/lib/python3.12/site-packages/cattrs/strategies/_unions.py", line 54, in configure_tagged_union
    args = union.__args__
           ^^^^^^^^^^^^^^
AttributeError: type object 'Sub' has no attribute '__args__'

Analysis: in _include_subclasses_with_union_strategy the (autogenerated) union_classes contains Sub2 twice (it has the value union_classes = (<class '__main__.Base'>, <class '__main__.Mid1'>, <class '__main__.Sub'>, <class '__main__.Mid2'>, <class '__main__.Sub'>)). When the loop for cl in union_classes gets to cl=Sub2 it creates subclasses=(Sub2, Sub2), passes the test len(subclasses) > 1, but then the Union automatically reduces this to a non-union Sub2 which has no __args__.

I think _make_subclasses_tree should return a distinct list that doesn't include any class twice.

[danarmak]

This is not cattrs fault! It's because attrs uses slots by default, and creates a new class to replace the original Sub, so the old and new Sub are both in the __subclasses__ list. Disabling slots solves the issue.

This was quite confusing; I don't know if cattrs can handle this better, so I'll leave the ticket open.

[danarmak]

No, I was wrong: this is a real bug in cattrs. I can trigger it by using more diamond inheritance and so causing _make_subclasses_subtree to return the same class twice. Here's an updated example that fails without using slots classes:

import cattrs.strategies
from attrs import frozen

@frozen(slots=False)
class Base: pass
@frozen(slots=False)
class Mid1(Base): pass
@frozen(slots=False)
class Mid2(Base): pass
@frozen(slots=False)
class Sub(Mid1, Mid2): pass
@frozen(slots=False)
class Sub2(Mid1, Mid2): pass

converter = cattrs.Converter()
cattrs.strategies.include_subclasses(Base, converter, union_strategy=cattrs.strategies.configure_tagged_union)

When not using slots classes, I can work around the bug by passing subclasses=tuple(set(_make_subclasses_tree(Base))) to strategies.include_subclasses.

[Tinche]

Hi, I was this issue when you created it and made a mental note to comment, and then promptly forgot. Apologies!

I think I have the solution in the linked PR, which will be merged soon.

[danarmak]

Great! Thank you. This will let me remove the workaround in my codebase.