Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Problem with tagged union example #533

Closed
RazerM opened this issue Apr 18, 2024 · 3 comments · Fixed by #534
Closed

Problem with tagged union example #533

RazerM opened this issue Apr 18, 2024 · 3 comments · Fixed by #534
Labels
regression This is an inintended change for the worse.
Milestone

Comments

@RazerM
Copy link

RazerM commented Apr 18, 2024

  • cattrs version: 23.2.3
  • Python version: 3.9-3.12
  • Operating System: macOS

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
Copy link
Member

Tinche commented Apr 18, 2024

Hm, confirming. Looks like this was broken in #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 Tinche added this to the 24.1 milestone Apr 18, 2024
@Tinche Tinche added the regression This is an inintended change for the worse. label Apr 18, 2024
@Tinche Tinche linked a pull request Apr 18, 2024 that will close this issue
@Tinche
Copy link
Member

Tinche commented Apr 18, 2024

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
Copy link
Author

RazerM commented Apr 18, 2024

Thanks!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
regression This is an inintended change for the worse.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants