lovasoa / marshmallow_dataclass

Automatic generation of marshmallow schemas from dataclasses.
https://lovasoa.github.io/marshmallow_dataclass/html/marshmallow_dataclass.html
MIT License
458 stars 77 forks source link

Union no longer passes metadata to component fields #166

Open dcchut opened 3 years ago

dcchut commented 3 years ago

The following snippet runs successfully using marshmallow_dataclass[enum,union]==8.5.2:

import enum
from typing import Union

import marshmallow_dataclass
from dataclasses import dataclass, field

class Fruit(enum.Enum):
    apple = "Apple"
    banana = "Banana"
    tomato = "Tomato"

@dataclass
class Punnet:
    fruit: Union[Fruit, dict] = field(metadata=dict(by_value=True))

schema = marshmallow_dataclass.class_schema(Punnet)()
assert schema.load({"fruit": "Tomato"}) == Punnet(fruit=Fruit.tomato)

As of 8.5.3 this no longer runs, instead raising an error:

marshmallow.exceptions.ValidationError: {'fruit': [ValidationError('Invalid enum member Tomato',), ValidationError('Not a valid mapping type.',)]}

I believe this was caused by the change in #162 (previously the by_value flag was being passed to the enum field).

gnedivad commented 2 years ago

Can we update _field_for_generic_type to pass through the metadata [1] i.e. update that line to be

metadata={**metadata, "required": True}

[1] https://github.com/lovasoa/marshmallow_dataclass/blob/6b6dc51c528e7c859a04f80c8af97c9cc1850bdb/marshmallow_dataclass/__init__.py#L582

dairiki commented 1 year ago

Can we update _field_for_generic_type to pass through the metadata [1] i.e. update that line to be

metadata={**metadata, "required": True}

This suggestion here is to pass (all or most) of the metadata specified for the Union field on to the component fields of the union. That feels fishy to me, as there's no way to control which of the subfields receives which metadata. What metadata is meant for the Union field, what is meant for the enum subfield, and what is meant for the dict subfield? It seems unlikely that we really want all three of those to receive the same metadata, at least in the general case. (Here we're passing by_value=True on to the Mapping field for the dict as well — that's harmless in this particular case, but not always.)

Issue #167 is related, having to do with how to pass metadata to subfields of a List field.


Here's one way around the issue, using marshmallow_dataclass.NewType to specify metadata for the enum subfield.

import enum
from typing import Union

from dataclasses import dataclass
from marshmallow_dataclass import class_schema, NewType

class Fruit(enum.Enum):
    apple = "Apple"
    banana = "Banana"
    tomato = "Tomato"

Fruit_by_value = NewType("Fruit_by_value", Fruit, by_value=True)

@dataclass
class Punnet:
    fruit: Union[Fruit_by_value, dict[str, str]]

schema = class_schema(Punnet)()
assert schema.load({"fruit": "Tomato"}) == Punnet(fruit=Fruit.tomato)

Spitballing: Maybe we could add a way to specify metadata for the Union field's subfields? Something like


@dataclass
class Punnet2:
    fruit: Union[Fruit, dict] = field(metadata={"subfield_metadata": [{"by_value": True}, {}]})

where subfield_metadata is an iterable of metadata to use with the union fields subfields (so, in this case, the {"by_value": True} goes to the Fruit subfield, and the {} goes to the Mapping subfield).

This could also be applied to other nested types.


@dataclass
class Punnet3:
     nonegs: List[int] = field(metadata={"subfield_metadata": [{"validate": validators.Range(0)}]})
     deepnest: List[Tuple[int, ...]] = field(
         metadata={
            "subfield_metadata": [{"subfield_metadata": [{"validate": validators.Range(0)}]}]
         }
      )

That seems ugly. Maybe someone can come up with something a bit more elegant.

dairiki commented 1 year ago

Spitballing: Maybe we could add a way to specify metadata for the Union field's subfields?

Here's another spitball. Maybe we could use PEP 593 annotations to attach metadata to nested types? Something like:

from dataclasses import dataclass
from typing import Annotated, List

from marshmallow import validators
from marshmallow_dataclass.annotations import FieldMetadata as Meta
@dataclass
class Punnet3:
     nonegs: List[
         Annotated[int, Meta(validate=validators.Range(0))]
     ]

Note Note that this syntax is not yet supported by marshmallow_dataclass — this is just an idea. (If there's interest I can try to implement it.)