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

Error when loading a field with typing.Any and value None #96

Open
sveinse opened this issue Mar 27, 2020 · 5 comments
Open

Error when loading a field with typing.Any and value None #96

sveinse opened this issue Mar 27, 2020 · 5 comments

Comments

@sveinse
Copy link
Collaborator

sveinse commented Mar 27, 2020

The following test generates an Validation error when the type hint typing.Any is used for a field and the de-serializer is given the value of None. From reading the py docs, I think the typing.Any hint intends to also cover None. The current workaround is to use t.Optional[t.Any].

Is this perhaps a Marshmallow issue?

def test_typing_any(module):
    """Test typing.Any

    """

    @module.dataclass
    class A:
        x: t.Any

    schema = desert.schema_class(A)()
    dumped = {"x": None}
    loaded = A(x=None)

    assert schema.load(dumped) == loaded
@python-desert python-desert added the bug Something isn't working label Mar 27, 2020
@python-desert
Copy link
Collaborator

python-desert commented Mar 27, 2020

Thanks @sveinse.

This raises a question. Currently t.Any goes to mm.fields.Raw. If we think that's okay, allow_none=True needs to be set on the field when the hint is t.Any. If you want to add

if typ == t.Any:
    field.allow_none = True

we can do that.

But I'm not certain that Any -> Raw mapping makes sense in the first place. Perhaps we should require passing a field explicitly here. This is a lot like the untagged union situation but with all types allowed instead of just the ones listed in t.Union[...].

@python-desert python-desert added design discussion and removed bug Something isn't working labels Mar 27, 2020
@sveinse
Copy link
Collaborator Author

sveinse commented Mar 27, 2020

In the context of serializers, Any is kinda pointless really. It is likely that the serializer can encode it proper, but the de-serializer might not decode the input correctly. You get what the transport format offers. In the case of JSON, then you basically get one of None, list, dict, int or float. Hence if the purpose is the be robust against such types, this links to the union selection discussions perhaps.

The reason I encountered this bug is because I have a data class with an extra internal field which is excluded from the schema. However due to #97 I must declare an arbitrary typing hint on this field.

@sveinse
Copy link
Collaborator Author

sveinse commented Apr 24, 2022

This issue is also visible in container uses, such as list and tuples:

@dataclasses.dataclass
class Data:
    x: List[Any]

which also results in deserialization error Field may not be null. The workaround is as mentioned using:

@dataclasses.dataclass
class Data:
    x: List[Optional[Any]]

However it doesn't feel elegant having to use the typing hint [Optional[Any]]. Are we any closer determining an opinion on using Any type hints in desert? I'm dependent on using Any in my application (as another field carries the selector for what type the field truly carries). However, if marshmallow doesn't really support Any maybe desert should raise an exception if it encounters the type hint?

@altendky
Copy link
Member

The first thing that comes to my mind for your containers is that they should be generic like other containers and thus instances would actually have a specific type. Though, that probably doesn't really solve anything. The generic form where you haven't figured out what is in it would I guess be object. I'm sure there's a lot to catch up on in terms of your particular use though so these are super generic (heh) thoughts.

My gut says that if we are mapping Any to something then we should allow None as well, but yeah, it's not clear to me that this is a thing that should work to begin with.

@sveinse
Copy link
Collaborator Author

sveinse commented Apr 25, 2022

It might be a little OT for the issue, but please let me give an overview of why I think we must consider this: The reason for having List[Any] or Tuple[Any, ...] in my use case is that I seem to be unable to represent the following typing in desert:

POS = Union[Tuple[str, str, float], str, None]
pan: Tuple[POS, POS]

I.e. the POS element of pan has three legal values: (1) None, (2) a string value e.g. 'R' or (3) a tuple ('L', 'R', 0.5). Serialization into json work fine, and since these three values have different json data type representations, deserialization from json into dict also works fine. However, I have not been able to find a way to represent the typing hints that desert is able to accept. The above typing does not currently work. Desert throws an exception on building the schema. Using pan: Tuple[Optional[Any],...] does work, although unspecific.

It might be argued that this is mapping too many variants into the same field. I haven't really found a good way to represent this type of composite object. One method might be to decompose the variants into multiple fields:

pan_if_2: Optional[str]
pan_if_3: Optional[Tuple(str, str, float)]

I'm not sure I find this elegant. In this application pan is an atomic object that can have three main states as given by the three variants (None, str or Tuple). At one point or another one have to deal with that this object has multiple representations. Optional[Any] works for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants