python-attrs / cattrs

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

Problem with tagged union example #533

Closed RazerM closed 5 months ago

RazerM commented 5 months ago

Description

I was following the docs on tagged unions in https://github.com/python-attrs/cattrs/blob/main/docs/strategies.md

What I Did

I'm unable to get OtherAppleNotification to work:

from typing import Union

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

@define
class Refund:
    originalTransactionId: str

@define
class OtherAppleNotification:
    notificationType: str

AppleNotification = Union[Refund, OtherAppleNotification]

c = Converter()
configure_tagged_union(
    AppleNotification,
    c,
    tag_name="notificationType",
    tag_generator={Refund: "REFUND"}.get,
    default=OtherAppleNotification,
)

payload = {"notificationType": "REFUND", "originalTransactionId": "1"}
notification = c.structure(payload, AppleNotification)
print(f"Structured {payload} as {notification}")

# All code above is from example

other = OtherAppleNotification(notificationType="OTHER")
payload = c.unstructure(other, AppleNotification)
print(f"Unstructured {other} as {payload}")

payload = {"notificationType": "OTHER"}
notification = c.structure(payload, AppleNotification)) # this line raises
print(f"Structured {payload} as {notification}")

I get the following output:

(1) This is given in the example and works fine:

Structured {'notificationType': 'REFUND', 'originalTransactionId': '1'} as Refund(originalTransactionId='1')

(2) Given the example I expected notificationType to be preserved, not None:

Unstructured OtherAppleNotification(notificationType='OTHER') as {'notificationType': None}

(3) I'm not able to structure a payload as OtherAppleNotification via the AppleNotification union:

  + Exception Group Traceback (most recent call last):
  |   File "scratch_175.py", line 40, in <module>
  |     print(c.structure(payload, AppleNotification))
  |   File "python3.9/site-packages/cattrs/converters.py", line 332, in structure
  |     return self._structure_func.dispatch(cl)(obj, cl)
  |   File "python3.9/site-packages/cattrs/converters.py", line 632, in _structure_union
  |     return handler(obj, union)
  |   File "python3.9/site-packages/cattrs/strategies/_unions.py", line 106, in structure_tagged_union
  |     return _tag_to_hook[val.pop(_tag_name)](val)
  |   File "python3.9/site-packages/cattrs/strategies/_unions.py", line 71, in structure_default
  |     return _h(val, _cl)
  |   File "<cattrs generated structure __main__.OtherAppleNotification>", line 9, in structure_OtherAppleNotification
  |     if errors: raise __c_cve('While structuring ' + 'OtherAppleNotification', errors, __cl)
  | cattrs.errors.ClassValidationError: While structuring OtherAppleNotification (1 sub-exception)
  +-+---------------- 1 ----------------
    | Traceback (most recent call last):
    |   File "<cattrs generated structure __main__.OtherAppleNotification>", line 5, in structure_OtherAppleNotification
    |     res['notificationType'] = __c_structure_notificationType(o['notificationType'])
    | KeyError: 'notificationType'
    | Structuring class OtherAppleNotification @ attribute notificationType
    +------------------------------------
Tinche commented 5 months ago

Hm, confirming. Looks like this was broken in https://github.com/python-attrs/cattrs/pull/443.

That PR pops out the tag out of the payload to make the strategy work with forbid_extra_keys. But it does feel useful to be able to have the tag.

Tinche commented 5 months ago

Thanks, got it fixed. I also turned the example into a doctest so we don't get into the same situation in the future.

RazerM commented 5 months ago

Thanks!