cattrs icon indicating copy to clipboard operation
cattrs copied to clipboard

Can't use disambiguation strategy together with `Optional`

Open MicaelJarniac opened this issue 2 years ago • 4 comments

  • cattrs version: 23.1.0.dev0
  • Python version: 3.11.1
  • Operating System: Manjaro KDE

Description

An optional tagged union doesn't work.

I believe the Optional (or | None) portion could be parsed beforehand, like it is on most fields, and the remaining type (Foo | Bar) then parsed separately, thus recognizing it as a tagged union.

What I Did

import attrs
import cattrs
from cattrs.strategies import configure_tagged_union


@attrs.define
class Foo:
    n: int = 42


@attrs.define
class Bar:
    n: int = 0


@attrs.define
class Baz:
    c: Foo | Bar


@attrs.define
class BazNone:
    c: Foo | Bar | None  # Could also have `= None` as a default value


converter = cattrs.Converter()
configure_tagged_union(Foo | Bar, converter=converter)
print(converter.unstructure(Baz(Bar(123))))  # {'c': {'n': 123, '_type': 'Bar'}}
print(converter.unstructure(BazNone(Bar(123))))  # {'c': {'n': 123}}

MicaelJarniac avatar Mar 29 '23 13:03 MicaelJarniac

I agree, but we should go even further: both union strategies should support None, literals and enums in addition to attrs classes, to enable proper sum type handling.

However it's not trivial, so it'll have to wait until 23.2.0 probably.

Tinche avatar Mar 29 '23 22:03 Tinche

Specifically for None, I managed to write something that seems to help a bit:

from typing import Any, Union

import cattrs
from cattrs._compat import get_args, is_union_type  # type: ignore

def configure_converter_none(converter: cattrs.Converter) -> None:
    # https://github.com/python-attrs/cattrs/issues/178
    c = converter

    NoneType = type(None)

    def has_none(t: Any) -> bool:
        return is_union_type(t) and NoneType in get_args(t)

    def without_none(t: Any) -> Any:
        return Union[tuple(a for a in get_args(t) if a is not NoneType)]  # type: ignore

    c.register_unstructure_hook_factory(
        has_none,
        lambda t: lambda v: c.unstructure(v, None if v is None else without_none(t)),
    )

    c.register_structure_hook_func(
        has_none,
        lambda v, t: None if v is None else c.structure(v, without_none(t)),
    )

I can use it like so:

import attrs

from cattrs.strategies import configure_tagged_union


@attrs.define
class Foo:
    n: int = 42


@attrs.define
class Bar:
    n: int = 0


@attrs.define
class BazNone:
    c: Foo | Bar | None


c = cattrs.Converter()
configure_tagged_union(Foo | Bar, converter=c)
configure_converter_none(c)

a = BazNone(Foo(123))  # BazNone(c=Foo(n=123))
b = BazNone(None)  # BazNone(c=None)
ad = c.unstructure(a)  # {'c': {'n': 123, '_type': 'Foo'}}
bd = c.unstructure(b)  # {'c': None}
ar = c.structure(ad, BazNone)  # BazNone(c=Foo(n=123))
br = c.structure(bd, BazNone)  # BazNone(c=None)

This is very far from ideal, but it seems to do the trick, from the limited testing I did.

This is based on your suggestion for #178.

By the way, if you know what are the proper type hints to use instead of Any, know how to get rid of the # type: ignores, or have any suggestions as to how to improve this, I'd really appreciate it. I still use some very similar code.

MicaelJarniac avatar Mar 31 '23 14:03 MicaelJarniac

The following seems to kinda work for what I want, but I believe there are many bugs and the performance is probably pretty bad:

from enum import Enum
from typing import Any, Union, get_args

import attrs
import cattrs
from cattrs._compat import is_union_type
from cattrs.strategies import configure_tagged_union


class Missing(Enum):
    MISSING = "!MISSING!"


MISSING = Missing.MISSING


@attrs.define
class Foo:
    n: int = 42


@attrs.define
class Bar:
    n: int = 0


@attrs.define
class Baz:
    c: Foo | Bar


@attrs.define
class BazNone:
    c: Foo | Bar | None


@attrs.define
class BazMissing:
    c: Foo | Bar | Missing = MISSING


def configure_converter_sentinels(
    converter: cattrs.Converter, sentinels: set[Any]
) -> None:
    # https://github.com/python-attrs/cattrs/issues/345
    # https://github.com/python-attrs/cattrs/issues/178
    c = converter

    # https://github.com/python-attrs/cattrs/issues/346
    if None in sentinels:
        sentinels.remove(None)
        sentinels.add(type(None))

    if type(None) in sentinels:
        def structure_none(v: Any, t: Any) -> None:
            if v is None:
                return v
            raise ValueError
        c.register_structure_hook(type(None), structure_none)

    def has_sentinels(t: Any) -> bool:
        return is_union_type(t) and any(a in sentinels for a in get_args(t))

    def is_sentinel(v: Any) -> bool:
        return any(isinstance(v, s) for s in sentinels)

    def without_sentinels(t: Any) -> Any:
        return Union[tuple(set(get_args(t)) - sentinels)]  # type: ignore

    def only_sentinels(t: Any) -> Any:
        return Union[tuple(set(get_args(t)) & sentinels)]  # type: ignore

    c.register_unstructure_hook_factory(
        has_sentinels,
        lambda t: lambda v: c.unstructure(
            v, None if is_sentinel(v) else without_sentinels(t)
        ),
    )

    def structure(v: Any, t: Any) -> Any:
        try:
            return c.structure(v, only_sentinels(t))
        except ValueError:
            return c.structure(v, without_sentinels(t))

    c.register_structure_hook_func(
        has_sentinels,
        structure,
    )


c = cattrs.Converter()

configure_tagged_union(Foo | Bar, converter=c, default=Foo)
configure_converter_sentinels(c, {Missing, None})


def roundtrip(v) -> None:
    print(f"In:    {v}")
    d = c.unstructure(v)
    print(f"Dict:  {d}")
    r = c.structure(d, type(v))
    print(f"Out:   {r}")
    print(f"Match: {v == r}")
    print()


roundtrip(BazMissing(Foo(123)))
roundtrip(BazMissing(MISSING))
roundtrip(BazNone(Bar(321)))
roundtrip(BazNone(None))
In:    BazMissing(c=Foo(n=123))
Dict:  {'c': {'n': 123, '_type': 'Foo'}}
Out:   BazMissing(c=Foo(n=123))
Match: True

In:    BazMissing(c=<Missing.MISSING: '!MISSING!'>)
Dict:  {'c': '!MISSING!'}
Out:   BazMissing(c=<Missing.MISSING: '!MISSING!'>)
Match: True

In:    BazNone(c=Bar(n=321))
Dict:  {'c': {'n': 321, '_type': 'Bar'}}
Out:   BazNone(c=Bar(n=321))
Match: True

In:    BazNone(c=None)
Dict:  {'c': None}
Out:   BazNone(c=None)
Match: True

MicaelJarniac avatar Apr 03 '23 17:04 MicaelJarniac

Hi,

so about advice on how to get rid of #type: ignores, unfortunately the Python type system doesn't have good support for this kind of metaprogramming yet.

Now, as for adding support for None to tagged unions, here's how I would go about it if I were you.

from typing import Any, Union, get_args

import attrs

import cattrs
from cattrs.strategies import configure_tagged_union


@attrs.define
class Foo:
    n: int = 42


@attrs.define
class Bar:
    n: int = 0


@attrs.define
class BazNone:
    c: Foo | Bar | None


def my_configure_tagged_union(union: Any, converter: cattrs.Converter, default: Any):
    if type(None) in get_args(union):
        union_without_none = Union[*(t for t in get_args(union) if t is not type(None))]
        configure_tagged_union(union_without_none, converter)
        struct_hook = converter._structure_func.dispatch(union_without_none)

        def my_struct_hook(val: Any, t: Any):
            if val is None:
                return None
            return struct_hook(val, union_without_none)

        c.register_structure_hook(union, my_struct_hook)

        unstruct_hook = converter._unstructure_func.dispatch(union_without_none)

        def my_unstruct_hook(val):
            if val is None:
                return None
            return unstruct_hook(val)

        c.register_unstructure_hook(union, my_unstruct_hook)

    else:
        configure_tagged_union(union, converter)


c = cattrs.Converter()

my_configure_tagged_union(Foo | Bar | None, c, Foo)


def roundtrip(v) -> None:
    print(f"In:    {v}")
    d = c.unstructure(v)
    print(f"Dict:  {d}")
    r = c.structure(d, type(v))
    print(f"Out:   {r}")
    print(f"Match: {v == r}")
    print()


roundtrip(BazNone(Bar(321)))
roundtrip(BazNone(None))

This essentially applies a wrapper on top of the tagged union strategy, dealing with possible Nones.

Cattrs was designed to be flexible in this way, hopefully allowing you to help yourself ;)

Tinche avatar Apr 04 '23 00:04 Tinche