Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

cattrs does not adhere to PEP 593 #127

Closed
Dr-ZeeD opened this issue Mar 6, 2021 · 7 comments
Closed

cattrs does not adhere to PEP 593 #127

Dr-ZeeD opened this issue Mar 6, 2021 · 7 comments

Comments

@Dr-ZeeD
Copy link

Dr-ZeeD commented Mar 6, 2021

  • cattrs version: 1.3.0
  • Python version: 3.9.1
  • Operating System: macOS

Description

PEP 593 introduced Annotated in typing.
According to this PEP

If a library (or tool) encounters a typehint Annotated[T, x] and has no special logic for metadata x, it should ignore it and simply treat the type as T.
Thus, I expect cattrs to work without any issues when I use Annotated. However, that is not the case. In the following, I provide a minimal working example outlining what should happen and what actually happens.

What I Did

from typing import Annotated

from attr import define

from cattr import GenConverter

converter = GenConverter()


@define
class A:
    a: int


@define
class B:
    b: A


@define
class C:
    c: Annotated[A, "test"]


if __name__ == "__main__":
    a = A(a=0)

    # Test it without an annotation
    b = B(b=a)
    b_unstructured = converter.unstructure(b)
    b_expected = {"b": {"a": 0}}
    print(b_unstructured)
    print(b_expected)

    # Test it with an annotation
    c = C(c=a)
    c_unstructured = converter.unstructure(c)
    c_expected = {"c": {"a": 0}}
    print(c_unstructured)  # {'c': A(a=0)}
    print(c_expected)  # 'c': {'a': 0}}
@Dr-ZeeD
Copy link
Author

Dr-ZeeD commented Mar 7, 2021

I have just found a workaround:

from typing import get_origin


def can_handle(cls):
    return issubclass(get_origin(cls), Annotated)


def unstructure(annotated):
    return converter.unstructure(annotated)


converter.register_unstructure_hook_func(can_handle, unstructure)

@Tinche
Copy link
Member

Tinche commented Mar 7, 2021

Yep, we don't support Annotated yet. We probably should at some point since it's an interesting feature.

Your workaround is good, but it won't cover certain cases - it'll unstructure according to the runtime type of something, but unstructuring an attrs class actually unstructures the fields according to their type annotation. For example you might have a hook registered for Sequence[str], and a field annotated with Annotated[Sequence[str], "test"] wouldn't trigger using your code (since the actual runtime type of whatever is in there is most likely list or tuple).

I'm attaching a solution to handle this properly if you're in a hurry (using some internal stuff):

from typing import Annotated, get_origin
from functools import partial

from attr import define

from cattr import GenConverter

converter = GenConverter()


@define
class A:
    a: int


@define
class B:
    b: A


@define
class C:
    c: Annotated[A, "test"]


a = A(a=0)
c = C(c=a)


def can_handle(cls):
    return issubclass(get_origin(cls), Annotated)


def gen_annotated_unstructure(annotated_type):
    return partial(
        converter.unstructure, unstructure_as=annotated_type.__args__[0]
    )


converter._unstructure_func.register_func_list(
    [(can_handle, gen_annotated_unstructure, True)]
)
c_unstructured = converter.unstructure(c)
c_expected = {"c": {"a": 0}}
print(c_unstructured)  # {'c': A(a=0)}
print(c_expected)  # 'c': {'a': 0}}

@Dr-ZeeD
Copy link
Author

Dr-ZeeD commented Mar 8, 2021

Thank you very much for this! I think if Annotated is to be considered in cattrs, it is an interesting question of "where to plug it in". What I mean is: Only fall back to serializing it as the first argument of the Annotated attribute if the user did not register a hook that wants to work with them.

@Tinche
Copy link
Member

Tinche commented Mar 8, 2021

Ah, you mean support hooks for Annotated[str, "test"] directly, and then fall back to str? Interesting.

@Dr-ZeeD
Copy link
Author

Dr-ZeeD commented Mar 8, 2021

Yup. Annotations could be a means to modify the runtime behavior of (un-)structuring. Example: Adding a "DoNotUnstructure" sentinel for annotating fields that should not be unstructured, i.e., not shown when unstructuring. It does not interfere with the typing system and should not disturb anything really, as every library should just ignore annotations it doesn't know.

Of course I do not really want that as a feature per se, but this is where I see Annotated going. Take attrs. Maybe we won't need "field" in the future because everything is done in Annotated.

@Tinche
Copy link
Member

Tinche commented Mar 14, 2021

Preliminary support added here: https://github.com/Tinche/cattrs/tree/feature/annotated

@Tinche
Copy link
Member

Tinche commented Mar 21, 2021

Released as 1.4.0.

@Tinche Tinche closed this as completed Mar 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants