python-attrs / cattrs

Composable custom class converters for attrs, dataclasses and friends.
https://catt.rs
MIT License
799 stars 111 forks source link

`configure_tagged_union` doesn't mix with `forbid_extra_keys=True` #402

Closed twm closed 10 months ago

twm commented 1 year ago

Description

I tried to mix cattrs.strategies.configure_tagged_union with a converter configured with forbid_extra_keys=True. Ideally this would work out of the box — structuring would remove the _type key before structuring the specific union member. Failing that this should be mentioned in the docs.

What I Did

Given tagged_union.py:

import attrs
from cattrs import Converter
from cattrs.strategies import configure_tagged_union

@attrs.define
class A:
    pass

@attrs.define
class B:
    pass

c = Converter(forbid_extra_keys=True)
configure_tagged_union(A | B, c)

data = c.unstructure(A(), A | B)
print(data)
c.structure(data, A | B)

I get an exception because of the _type keys that the tagged union strategy injects:

❯ python tagged_union.py 
{'_type': 'A'}
  + Exception Group Traceback (most recent call last):
  |   File ".../tagged_union.py", line 21, in <module>
  |     c.structure(data, A | B)
  |   File ".../venv/lib/python3.10/site-packages/cattrs/converters.py", line 334, in structure
  |     return self._structure_func.dispatch(cl)(obj, cl)
  |   File ".../venv/lib/python3.10/site-packages/cattrs/converters.py", line 651, in _structure_union
  |     return handler(obj, union)
  |   File ".../venv/lib/python3.10/site-packages/cattrs/strategies/_unions.py", line 86, in structure_tagged_union
  |     return _tag_to_cl[val[_tag_name]](val)
  |   File ".../venv/lib/python3.10/site-packages/cattrs/strategies/_unions.py", line 52, in structure_union_member
  |     return _h(val, _cl)
  |   File "<cattrs generated structure __main__.A>", line 7, in structure_A
  |     if errors: raise __c_cve('While structuring ' + 'A', errors, __cl)
  | cattrs.errors.ClassValidationError: While structuring A (1 sub-exception)
  +-+---------------- 1 ----------------
    | cattrs.errors.ForbiddenExtraKeysError: Extra fields in constructor for A: _type
    +------------------------------------
Tinche commented 1 year ago

Hm we can probably just fix it so it pops the discriminant key out. I don't think the current behavior is on purpose.