Repository: seandstewart/typical (language: python)

Issue:
Coercion to Union does not result in error but returns passed in value -- Union could be expanded
It's stated in the README:

> The following annotations are special forms which cannot be resolved:
> Union
> Any
> Because these signal an unclear resolution, Typical will ignore this flavor of annotation ...

However, when I try to coerce some values to Unions, I didn't expect Typical to *silently* ignore Union annotations: (e.g. it returns a `str` when a `str` is passed in, when none of the Union members are `str`). Since it is ignored, I can't rely on passing my value through coercion to actually get the correct type returned to me. I think this is a major limitation and a source of errors.

```python
from typing import Union
import typic


# Case A. Incorrect. Expected: ValueError
print(typic.coerce("foo", Union[float, int]))  # foo

from enum import Enum

class MyEnum(Enum):
  a = "foo"
  b = "bar"

# Case C1. Correct: Union with None works.
print(typic.coerce("foo", Union[MyEnum, None]))  # MyEnum.a
print(typic.coerce(None, Union[MyEnum, None]))  # None

# Case C1. Incorrect. Expected: MyEnum.a
print(typic.coerce("foo", Union[MyEnum, int]))  # foo
print(typic.coerce("foo", Union[MyEnum, bool]))  # foo

class Sentinel:
  pass

# Case B. Incorrect. Expected: MyEnum.a
print(typic.coerce("foo", Union[MyEnum, Sentinel]))  # foo

# Case C1: Correct: The value is an instance of one of the types
print(typic.coerce(1, Union[bool, int]))  # 1
print(typic.coerce(False, Union[bool, int]))  # False

# Case B. Incorrect. Expected: True (since it can be coerced to bool, but not to int)
print(typic.coerce("asdf", Union[bool, int]))  # asdf

# Case C2. Expected: TypeError, since it could be coerced to both
print(typic.coerce("1", Union[bool, int]))  # 1


```

Would it really be problematic to handle coercing to Union in some cases?

I can imagine scenarios where it's unambiguous, and one scenario where it is and an error could be raised:
- A. If the value cannot be coerced to any of the types: raise one of the coercion errors (e.g. the last)
- B. If the value can be coerced to only one of the types (the others may raise ValueError/TypeError): return that coerced value
- C. If the value can be coerced to multiple of the types:
  - C1: If the value is an instance of one of the union's types, then "coerce" it to that type -- coerce in this context means just perform any constraint checks that there might be (This coercion is already handled for `Union[T, None]`)
  - C2: Otherwise: Raise a TypeError (this is the ambiguous scenario)

What do you think?

In my mind, it might look something like this:

```python
from typing import Any, Iterable, Type
from typic import coerce

# Sentinel value
class Empty:
    pass


_empty = Empty()


def coerce_union(value: Any, union: Iterable[Type]) -> Any:
    coerced_value = _empty

    for typ in union:
        # If the value is already an instance of one of the types in the Union, coerce using that type
        # Presumably, if the type was constrained, coercing could raise errors here, but that would be expected.
        if isinstance(value, typ):
            return coerce(value, typ)
    error = None

    for typ in union:
        try:
            new_coerced_value = coerce(value, typ)
        except (ValueError, TypeError) as e:
            # Store the error and continue
            error = e
            continue
        if coerced_value is not _empty:
            raise TypeError("Ambiguous coercion: the value could be coerced to multiple types in the Union")
        coerced_value = new_coerced_value
    if coerced_value is not _empty:
        return coerced_value
    # The value couldn't be coerced to any of the types in the Union
    raise error
```


Discussion / hints:
Thank you for the well-thought-out discussion on Unions. It's a bit of a weird case, since as I note in the documentation, the resolution path for a Union-type is unclear when it comes to coercion, since we don't actually know what the user wants. You do bring up a good point about the runtime validation of Union-types. 

We don't currently do any actual runtime validation beyond the attempt to coerce a type and I feel that this logic bleeds into validation logic rather than coercion logic, but I'll have to think about this a bit more.
You're welcome. I certainly understand the concern about not wanting to guess what the user wants and about adding validation logic.
I already see there is a potential issue in my proposal: In case C1, where you check if the type `isinstance` of one of the types in the Union, you could get multiple matches, due to subclassing: therefore that should also raise an error in the case of multiple valid coercions.

Still, I think ignoring coercion of Unions is not a good idea, because, as far as I understand the idea behind coercion, is that you are guaranteed that you get the target type back or an error is thrown. That's not the case right now for `typic.coerce("foo", Union[float, int])`. I'd prefer some error (e.g. TypeError) to be thrown there.

I wonder if we could make everyone happy by allowing to register a custom Union coercer, with the default being that an error is thrown? (unless it's this special case of `Optional`?)
Currently, it's possible to register a custom coercer for Union using `typic.register` (`typic.register(union_coercer, lambda typ: getattr(typ, "__origin__", typ) == Union)`), but it's not very useful... the coercer function does not get passed the type it's coercing to, so there would be no way to decide which type to coerce to. Therefore, I wouldn't be able to implement the kind of coercion that I outlined. Maybe you'd be willing to add an extra parameter to the coercer, passing the target type? Or maybe it should be a separate `register_union_coercer` function or somesuch...

So, if you don't feel inclined to make the library opinionated about this, maybe opening it up for the user to decide is a good choice.
By the way, my original use case was that I have a sentinel value that I default for all values in a class to indicate missing values, e.g.:

```python

class Missing:
    pass


MISSING = Missing()
@typic.al
@dataclass
class PartialBlogEntity:
    title: Union[Optional[str], Missing] = MISSING
```

In that case, I never want typical to coerce to the `Missing` type.
Here’s a complete example of registering a custom coercer from my library [iambic](https://github.com/seandstewart/iambic/blob/bf5313c8dbd43f87c2cc890eb96a228432bb5faf/iambic/ast/node.py#L496) which makes use of Unions.

In your case, I would probably write a check which looks for a Union as the origin and your `Missing` sentinel as an arg.
Thanks for the example. I can see how it works in this case where you essentially have a discriminated union (e.g. you can check the `"type"` field of the `value` to know what to coerce to). But I don't see how I could make this in a generic way, without registering a coercer for each possible Union combination.

For example, if I have more fields, like this:

```python
@typic.al
@dataclass
class PartialBlogEntity:
    title: Union[Optional[str], Missing] = MISSING
    created_at: Union[datetime, Missing] = MISSING
```

I would have to register a coercer with a check function that looked for `Union[Optional[str], Missing]`, which always coerced to `Optional[str]`; then, register another coercer with a check function that looked for `Union[datetime, Missing]` and always coerced to `datetime`.
Am I missing something?
You could call `typic.coerce` on the nested type within your own coercer.
@syastrov I believe I've provided a path for something like what you're asking for here in v2.0.0b8. That version should be released today.
I've been noodling on this a bit and decided it's a good thing to add to the library. We added support for ["tagged unions"](https://python-typical.org/usage/types/#tagged-unions-polymorphic-types) a while back, which solves the main use-case of custom type registration, but there are real cases where it may be useful to allow for this.

Fix patch:
diff --git a/docs/usage/types.md b/docs/usage/types.md
index 8b4fe384..6546bfb5 100644
--- a/docs/usage/types.md
+++ b/docs/usage/types.md
@@ -236,7 +236,13 @@ typic.transmute(SuperImportantValues, b"foo")
 ```
 
 
-## Tagged Unions (Polymorphic Types)
+## Unions (Polymorphic Types)
+
+!!! warning ""
+
+    :dragon: Here be dragons :dragon: 
+
+### Tagged Unions
 
 !!! info ""
 
@@ -435,6 +441,69 @@ print(member.play())
         field: bytes
     ```
 
+### Generic Unions
+
+While you're *highly encouraged* to make use of [Tagged Unions](#tagged-unions) for your
+polymorphic types, typical can generate a deserializer for generic unions as well. This
+is intended for use when it's simply not possible to define a discriminator for your
+union.
+
+!!! info ""
+
+    New in version 2.6
+
+!!! warning ""
+
+    Tagged Union deserialization is O(1) where N is the number of target types. Generic
+    Unions are O(N). Keep this in mind when defining your types - you may be better-served
+    by re-working your data model.
+
+When defining your Generic Union, you're encouraged to order your types from *most*
+specific to *least*. As a part of the implementation, we treat the possible types as
+FIFO queue, taking a type from the top of the stack and attempting deserialization. If
+all attempt at deserialization fail, we raise a `ValueError`.
+
+??? example "Working with Generic Unions"
+
+    **Wrong:**
+    
+    ```python
+    from __future__ import annotations
+    
+    from typing import Union
+    
+    import typic
+    
+    
+    # `str` should never be first! Everything can be a string...
+    proto = typic.protocol(Union[str, int])
+    print(type(proto.transmute("1")))
+    #> <class 'str'>
+    
+    ```
+    
+    **Right:**
+    
+    ```python
+    from __future__ import annotations
+    
+    from typing import Union
+    
+    import typic
+    
+    proto = typic.protocol(Union[int, str])
+    print(type(proto.transmute("1")))
+    #> <class 'int'>
+    ```
+
+!!! error "Gotcha!"
+
+    In static typing, `Union[str, int]` and `Union[int, str]` are identical. For Python,
+    this means they have the same hash value, which in turn breaks typical's caching 
+    mechanism. *Tread carefully when defining your types and always ensure you define 
+    your union from* most *to* least *strict.*
+
+
 ## Constraining Builtin Types
 Typical provides a path for defining "constrained" types based upon
 Python builtins. This gives you a means to express limited types in a
diff --git a/typic/serde/des.py b/typic/serde/des.py
index 36a41904..00f81507 100644
--- a/typic/serde/des.py
+++ b/typic/serde/des.py
@@ -36,6 +36,7 @@
     get_defname,
     get_tag_for_types,
     get_unique_name,
+    get_name,
 )
 from typic.common import DEFAULT_ENCODING, VAR_POSITIONAL, VAR_KEYWORD, ObjectT
 from .common import (
@@ -653,46 +654,73 @@ def _build_literal_des(
         )
         return self._build_des(t_anno, func_name, namespace)
 
-    def _build_union_des(self, func: gen.Block, annotation: Annotation, namespace):
+    def _build_tagged_union_des(
+        self, func: gen.Block, annotation: Annotation, namespace
+    ):
         # Get all types which we may coerce to.
         args = (*(a for a in annotation.args if a not in {None, Ellipsis, type(None)}),)
+        if not args:
+            return
         # Get all custom types, which may have discriminators
         targets = (*(a for a in args if not checks.isstdlibtype(a)),)
         # We can only build a tagged union deserializer if all args are valid
-        if args and args == targets:
-            # Try to collect the field which will be the discriminator.
-            # First, get a mapping of Type -> Proto & Type -> Fields
-            tagged = get_tag_for_types(targets)
-            # Just bail out if we can't find a key.
-            if not tagged:
-                func.l("# No-op, couldn't locate a discriminator key.")
-                return
-            # If we got a key, re-map the protocols to the value for each type.
-            deserializers = {
-                value: self.resolver.resolve(t, namespace=namespace)
-                for value, t in tagged.types_by_values
+        if args != targets:
+            return self._build_generic_union_des(func, annotation, namespace)
+
+        # Try to collect the field which will be the discriminator.
+        # First, get a mapping of Type -> Proto & Type -> Fields
+        tagged = get_tag_for_types(targets)
+        # Just bail out if we can't find a key.
+        if not tagged:
+            return self._build_generic_union_des(func, annotation, namespace)
+        # If we got a key, re-map the protocols to the value for each type.
+        deserializers = {
+            value: self.resolver.resolve(t, namespace=namespace).transmute
+            for value, t in tagged.types_by_values
+        }
+        # Finally, build the deserializer
+        func.namespace.update(
+            tag=tagged.tag,
+            desers=deserializers,
+            empty=_empty,
+        )
+        with func.b(f"if issubclass({self.VTYPE}, Mapping):", Mapping=abc.Mapping) as b:
+            b.l(f"tag_value = {self.VNAME}.get(tag, empty)")
+        with func.b("else:") as b:
+            b.l(f"tag_value = getattr({self.VNAME}, tag, empty)")
+        with func.b("if tag_value in desers:") as b:
+            b.l(f"{self.VNAME} = desers[tag_value]({self.VNAME})")
+        with func.b("else:") as b:
+            b.l(
+                "raise ValueError("
+                'f"Value is missing field {tag!r} with one of '
+                '{(*desers,)}: {val!r}"'
+                ")"
+            )
+
+    def _build_generic_union_des(
+        self, func: gen.Block, annotation: Annotation, namespace
+    ):
+        args = [a for a in annotation.args if a not in {None, Ellipsis, type(None)}]
+        if args:
+            ctx = {
+                get_name(a)
+                + "_des": self.resolver.resolve(a, namespace=namespace).transmute
+                for a in args
             }
-            # Finally, build the deserializer
-            func.namespace.update(
-                tag=tagged.tag,
-                desers=deserializers,
-                empty=_empty,
+            names = (*(n.rpartition("_")[0] for n in ctx),)
+            for name, call in ctx.items():
+                with func.b("try:") as b:
+                    b.l(f"return {name}({self.VNAME})")
+                with func.b("except (TypeError, ValueError, KeyError):") as b:
+                    b.l("pass")
+            func.namespace.update(ctx)
+            func.l(
+                "raise ValueError("
+                'f"Value could not be deserialized into one of {names}: {val!r}"'
+                ")",
+                names=names,
             )
-            with func.b(
-                f"if issubclass({self.VTYPE}, Mapping):", Mapping=abc.Mapping
-            ) as b:
-                b.l(f"tag_value = {self.VNAME}.get(tag, empty)")
-            with func.b("else:") as b:
-                b.l(f"tag_value = getattr({self.VNAME}, tag, empty)")
-            with func.b("if tag_value in desers:") as b:
-                b.l(f"{self.VNAME} = desers[tag_value].transmute({self.VNAME})")
-            with func.b("else:") as b:
-                b.l(
-                    "raise ValueError("
-                    'f"Value is missing field {tag!r} with one of '
-                    '{(*desers,)}: {val!r}"'
-                    ")"
-                )
 
     def _build_des(  # noqa: C901
         self,
@@ -720,7 +748,7 @@ def _build_des(  # noqa: C901
                 if origin not in self.UNRESOLVABLE:
                     self._set_checks(func, anno_name, annotation)
                     if checks.isuniontype(origin):
-                        self._build_union_des(func, annotation, namespace)
+                        self._build_tagged_union_des(func, annotation, namespace)
                     elif checks.isdatetype(origin):
                         self._build_date_des(func, anno_name, annotation)
                     elif checks.istimetype(origin):


Test patch:
diff --git a/tests/constraints/test_factory.py b/tests/constraints/test_factory.py
index db3ea5ac..30272e67 100644
--- a/tests/constraints/test_factory.py
+++ b/tests/constraints/test_factory.py
@@ -35,7 +35,7 @@ class MyURL(typic.URL):
         (frozenset, frozenset({})),
         (typing.Dict[str, int], {"foo": 1}),
         (typing.List[int], [1]),
-        (typing.Union[str, int], 1),
+        (typing.Union[int, str], 1),
         (typing.List[typing.Optional[typing.Dict[str, int]]], [None]),
         (typing.List[typing.Optional[typing.Dict[str, int]]], [{"foo": 1}]),
         (Foo, {"bar": "bar"}),
@@ -77,7 +77,7 @@ def test_get_contraints(t, v):
         (frozenset, ""),
         (typing.Dict[str, int], {"foo": ""}),
         (typing.List[int], [""]),
-        (typing.Union[str, int], []),
+        (typing.Union[int, str], []),
         (typing.List[typing.Optional[typing.Dict[str, int]]], [[]]),
         (typing.List[typing.Optional[typing.Dict[str, int]]], [{"foo": ""}]),
         (Foo, {"bar": 1}),
diff --git a/tests/test_schema.py b/tests/test_schema.py
index 38db47bc..17d91b8e 100644
--- a/tests/test_schema.py
+++ b/tests/test_schema.py
@@ -88,9 +88,9 @@ class Container:
         (Final[str], typic.StrSchemaField(readOnly=True)),
         (typic.common.WriteOnly[str], typic.StrSchemaField(writeOnly=True)),
         (
-            Union[str, int],
+            Union[int, str],
             typic.MultiSchemaField(
-                anyOf=(typic.StrSchemaField(), typic.IntSchemaField())
+                anyOf=(typic.IntSchemaField(), typic.StrSchemaField())
             ),
         ),
         (
@@ -152,18 +152,18 @@ class Container:
             ),
         ),
         (
-            Dict[str, Union[str, int]],
+            Dict[str, Union[int, str]],
             typic.ObjectSchemaField(
                 additionalProperties=MultiSchemaField(
-                    anyOf=(typic.StrSchemaField(), typic.IntSchemaField())
+                    anyOf=(typic.IntSchemaField(), typic.StrSchemaField())
                 )
             ),
         ),
         (
-            Tuple[Union[str, int], ...],
+            Tuple[Union[int, str], ...],
             typic.ArraySchemaField(
                 items=MultiSchemaField(
-                    anyOf=(typic.StrSchemaField(), typic.IntSchemaField())
+                    anyOf=(typic.IntSchemaField(), typic.StrSchemaField())
                 )
             ),
         ),
diff --git a/tests/test_typed.py b/tests/test_typed.py
index 320f7698..a08fa1d2 100644
--- a/tests/test_typed.py
+++ b/tests/test_typed.py
@@ -706,8 +706,8 @@ class Foo:
         (Strict[typing.Optional[typing.List[int]]], {None}),
         (Strict[typing.Optional[typing.List[int]]], [1.0]),
         (typing.Optional[Strict[str]], 1.0),
-        (Strict[typing.Union[str, int]], 1.0),
-        (Strict[typing.Union[str, int]], None),
+        (Strict[typing.Union[int, str]], 1.0),
+        (Strict[typing.Union[int, str]], None),
         (StrictStrT, b""),
     ],
     ids=objects.get_id,
@@ -727,8 +727,8 @@ def test_strict_anno_fails(anno, val):
         (typing.Optional[Strict[str]], "foo"),
         (Strict[typing.Optional[str]], "foo"),
         (Strict[typing.Optional[str]], None),
-        (Strict[typing.Union[str, int]], 1),
-        (Strict[typing.Union[str, int]], "foo"),
+        (Strict[typing.Union[int, str]], 1),
+        (Strict[typing.Union[int, str]], "foo"),
         (StrictStrT, "foo"),
     ],
     ids=objects.get_id,
@@ -1062,6 +1062,20 @@ def test_tagged_union_validate(annotation, value):
     assert validated == value
 
 
+@pytest.mark.parametrize(
+    argnames="annotation,value,expected",
+    argvalues=[
+        (typing.Union[int, str], "1", 1),
+        (typing.Union[int, str], "foo", "foo"),
+        (typing.Union[int, datetime.date], "1", 1),
+        (typing.Union[int, datetime.date], "1970-01-01", datetime.date(1970, 1, 1)),
+    ],
+)
+def test_union_transmute(annotation, value, expected):
+    transmuted = transmute(annotation, value)
+    assert transmuted == expected
+
+
 @pytest.mark.parametrize(
     argnames="annotation,value",
     argvalues=[