seandstewart / typical

Typical: Fast, simple, & correct data-validation using Python 3 typing.
https://python-typical.org
MIT License
183 stars 9 forks source link

Wrong behavior when Union with constrained types #170

Closed xbanke closed 2 years ago

xbanke commented 3 years ago

Description

In this version(2.6.3), it can handle Union if the given argument is just an instance of one of the types in the Union, but not support subtypes, checking type by issubclass or isinstance can sovle this. But if Union contains constrained types, the transmute function did not behave like we hope.

import typic, typing

PositiveFloat = typic.constrained(float, gt=0)
PositiveInt = typic.constrained(int, gt=0)

proto = typic.protocol(typing.Union[PositiveInt, PositiveFloat])

proto.transmute(PositiveFloat(2.1))  # 2.1
proto.transmute(2.1)  # 2
isinstance(2.1, PositiveFloat)  # False
issubclass(PositiveFloat, float)  # True
seandstewart commented 3 years ago

Hey @xbanke -

On the latest version, I'm seeing the correct behavior here:

>>> import typic, typing
... 
... PositiveFloat = typic.constrained(float, gt=0)
... PositiveInt = typic.constrained(int, gt=0)
... 
... proto = typic.protocol(typing.Union[PositiveFloat, PositiveInt])
>>> isinstance(proto.transmute("1.0"), PositiveFloat)
True
>>> isinstance(proto.transmute(1.0), PositiveFloat)
True
seandstewart commented 3 years ago

Note that mixing float and int are going to be problematic, since they're overlapping types. By that, I mean that in Python's runtime type system, float can be coerced to int and vice-versa without any sort of complaint.

From my testing, other libraries (e.g., pydantic, etc.) have the same behavior in this case. I'd consider this a limitation of Python's runtime type system more than an implementation issue in typical. Hate to pass the buck, but I don't see a good way to implement determinism in this case.

// edit:

You may consider taking advantage of the decimal module for these cases.

xbanke commented 3 years ago

Hey, @seandstewart . In your case PositiveFloat is at first place, all numbers will be converted to float, but in my case the PositiveInt is at the first, all numbers will be converted to int. I think it maybe should consider adding the father class for the constrainted types to choose the first converter. For example if given the value 1.2, it should first try to convert with PositiveFloat, even if the PositiveInt is the head.

One more thing, typical did not support abstract types that can not be instantiated.

import typic, numbers

@typic.al
def foo(a: numbers.Number):
    return a

foo(42)  # TypeError: Number() takes no arguments

Because it check type by is, but isinstance or issubclass.

proto = typic.protocol(numbers.Number)
print(proto.transmute.__raw__)
def deserializer__1872423425688238822(val):
    _, val = __eval(val) if isinstance(val, (str, bytes)) else (False, val)
    vtype = val.__class__
    if vtype is Number_94020002862272:
        return val
    # Happy path - deserialize a mapping into the object.
    if issubclass(vtype, Mapping):
        val = Number_94020002862272(**{x: desers[x](val[x]) for x in fields_in.keys() & val.keys()})
    # Unknown path, just try casting it directly.
    elif isbuiltinsubtype(vtype):
        val = Number_94020002862272(val)
    # Two user-defined types, try to translate the input into the desired output.
    else:
        val = translate(val, Number_94020002862272)
    return val

Here, the __raw__ shows that it check the type by if vtype is Number_94020002862272, not if isintance(val, Number_94020002862272) or issubclass(vtype, Number_94020002862272). This is simlar with Union

floriandorre commented 3 years ago

Hi,

I'm not sure if my issue is related to this topic but i also get an error when using Literal and Union

What i am doing :

from typing import Union

import typic
from typing_extensions import Literal

@typic.al
def dummy(arg_a: Union[float, Literal["A", "B"]] = "A"):
    return arg_a

if __name__ == '__main__':
    dummy()

Output :

Traceback (most recent call last):
  File "C:/Users/Florian DORRE/RTC/wastewater_toolkit/configuration/test.py", line 8, in <module>
    def dummy(arg_a: Union[str, Literal["A", "B"]] = "A"):
  File "C:\Users\Florian DORRE\Envs\ww-toolkit\lib\site-packages\typic\api.py", line 385, in typed
    return _typed(_cls_or_callable) if _cls_or_callable is not None else _typed
  File "C:\Users\Florian DORRE\Envs\ww-toolkit\lib\site-packages\typic\api.py", line 379, in _typed
    return wrap(obj, delay=delay, strict=strict)  # type: ignore
  File "C:\Users\Florian DORRE\Envs\ww-toolkit\lib\site-packages\typic\api.py", line 173, in wrap
    protocols(func)
  File "C:\Users\Florian DORRE\Envs\ww-toolkit\lib\site-packages\typic\serde\resolver.py", line 794, in protocols
    annotation, name=name, parameter=param, is_strict=strict, namespace=obj
  File "C:\Users\Florian DORRE\Envs\ww-toolkit\lib\site-packages\typic\serde\resolver.py", line 720, in resolve
    resolved = self._resolve_from_annotation(anno, namespace=namespace)
  File "C:\Users\Florian DORRE\Envs\ww-toolkit\lib\site-packages\typic\serde\resolver.py", line 533, in _resolve_from_annotation
    anno, constraints, namespace=namespace
  File "C:\Users\Florian DORRE\Envs\ww-toolkit\lib\site-packages\typic\serde\des.py", line 907, in factory
    deserializer = self._build_des(annotation, key, namespace)
  File "C:\Users\Florian DORRE\Envs\ww-toolkit\lib\site-packages\typic\serde\des.py", line 782, in _build_des
    deserializer = main.compile(ns=ns, name=func_name)
  File "C:\Users\Florian DORRE\Envs\ww-toolkit\lib\site-packages\typic\gen.py", line 191, in compile
    bytecode = compile(code, fname, "exec")
  File "<typical generated deserializer__2069376713718722944>", line 14
    return Literal['A', 'B']_des(val)
                               ^
SyntaxError: invalid syntax

NB: I am using python 3.7.6 so Literal comes from typing_extensions and not typing itself.

Thanks

seandstewart commented 2 years ago

Hey @xbanke, @floriandorre,

I've got PR #175 open which I believe resolves all the issues mentioned here:

If you could try your code against that branch and verify the output I'd really appreciate it!

floriandorre commented 2 years ago

This branch indeed fix my issue, Thanks !

xbanke commented 2 years ago

Yes, this branch fix my issue. But there is some other problem:

import typic, typing

proto = typic.protocol(typing.Union[list, typing.Dict[int, int]])
proto.transmute(1)  # [1, 0, 1, 1]

This looks likely related to #174

seandstewart commented 2 years ago

Yes, this branch fix my issue. But there is some other problem:

import typic, typing

proto = typic.protocol(typing.Union[list, typing.Dict[int, int]])
proto.transmute(1)  # [1, 0, 1, 1]

This looks likely related to #174

Yes, I think I know the issue - it has to do with our iterator factory:


import typic

[*typic.iterate(1)]

This factory will iterate over a class's fields if it is not natively iterable. Very useful for user-defined classes (e.g., data models), not so useful for primitives.

I will patch this up as well.