From 7a64b3c9d49ea7feb2bd7ff51668f7d7827cced1 Mon Sep 17 00:00:00 2001 From: Ritvik Nag Date: Mon, 25 Nov 2024 22:11:05 -0500 Subject: [PATCH 1/3] Fix: dataclasses in `Union` when `Meta.tag_key` is a dataclass field --- dataclass_wizard/class_helper.pyi | 44 ++++++++++++++----------------- dataclass_wizard/loaders.py | 2 +- tests/unit/test_load.py | 34 ++++++++++++++++++++++++ 3 files changed, 55 insertions(+), 25 deletions(-) diff --git a/dataclass_wizard/class_helper.pyi b/dataclass_wizard/class_helper.pyi index 0ff2cba5..c403aa4b 100644 --- a/dataclass_wizard/class_helper.pyi +++ b/dataclass_wizard/class_helper.pyi @@ -7,6 +7,7 @@ from .bases import META from .models import Condition from .type_def import ExplicitNullType, T from .utils.dict_helper import DictWithLowerStore +from .utils.object_path import PathType # A cached mapping of dataclass to the list of fields, as returned by @@ -31,8 +32,7 @@ CLASS_TO_DUMPER: dict[type, type[AbstractDumper]] = {} # A cached mapping of a dataclass to each of its case-insensitive field names # and load hook. -FIELD_NAME_TO_LOAD_PARSER: dict[ - type, DictWithLowerStore[str, AbstractParser]] = {} +FIELD_NAME_TO_LOAD_PARSER: dict[type, DictWithLowerStore[str, AbstractParser]] = {} # Since the dump process doesn't use Parsers currently, we use a sentinel # mapping to confirm if we need to setup the dump config for a dataclass @@ -40,12 +40,10 @@ FIELD_NAME_TO_LOAD_PARSER: dict[ IS_DUMP_CONFIG_SETUP: dict[type, bool] = {} # A cached mapping, per dataclass, of JSON field to instance field name -JSON_FIELD_TO_DATACLASS_FIELD: dict[ - type, dict[str, str | ExplicitNullType]] = defaultdict(dict) +JSON_FIELD_TO_DATACLASS_FIELD: dict[type, dict[str, str | ExplicitNullType]] = defaultdict(dict) # A cached mapping, per dataclass, of instance field name to JSON path -DATACLASS_FIELD_TO_JSON_PATH: dict[ - type, dict[str, list[str | int | bool | float]]] = defaultdict(dict) +DATACLASS_FIELD_TO_JSON_PATH: dict[type, dict[str, PathType]] = defaultdict(dict) # A cached mapping, per dataclass, of instance field name to JSON field DATACLASS_FIELD_TO_JSON_FIELD: dict[type, dict[str, str]] = defaultdict(dict) @@ -56,22 +54,20 @@ DATACLASS_FIELD_TO_SKIP_IF: dict[type, dict[str, Condition]] = defaultdict(dict) # A mapping of dataclass name to its Meta initializer (defined in # :class:`bases.BaseJSONWizardMeta`), which is only set when the # :class:`JSONSerializable.Meta` is sub-classed. -META_INITIALIZER: dict[ - str, Callable[[type[W]], None]] = {} - +META_INITIALIZER: dict[str, Callable[[type[W]], None]] = {} # Mapping of dataclass to its Meta inner class, which will only be set when # the :class:`JSONSerializable.Meta` is sub-classed. _META: dict[type, META] = {} -def dataclass_to_loader(cls: type): +def dataclass_to_loader(cls: type) -> type[AbstractLoader]: """ Returns the loader for a dataclass. """ -def dataclass_to_dumper(cls: type): +def dataclass_to_dumper(cls: type) -> type[AbstractDumper]: """ Returns the dumper for a dataclass. """ @@ -89,13 +85,13 @@ def set_class_dumper(cls: type, dumper: type[AbstractDumper]): """ -def json_field_to_dataclass_field(cls: type): +def json_field_to_dataclass_field(cls: type) -> dict[str, str | ExplicitNullType]: """ Returns a mapping of JSON field to dataclass field. """ -def dataclass_field_to_json_path(cls: type): +def dataclass_field_to_json_path(cls: type) -> dict[str, PathType]: """ Returns a mapping of dataclass field to JSON path. """ @@ -151,7 +147,7 @@ def _setup_load_config_for_cls(cls_loader: type[AbstractLoader], """ -def setup_dump_config_for_cls_if_needed(cls: type): +def setup_dump_config_for_cls_if_needed(cls: type) -> None: """ This function processes a class `cls` on an initial run, and sets up the dump process for `cls` by iterating over each dataclass field. For each @@ -172,7 +168,7 @@ def setup_dump_config_for_cls_if_needed(cls: type): """ -def call_meta_initializer_if_needed(cls: type[W]): +def call_meta_initializer_if_needed(cls: type[W]) -> None: """ Calls the Meta initializer when the inner :class:`Meta` is sub-classed. """ @@ -186,7 +182,7 @@ def get_meta(cls: type) -> META: """ -def dataclass_fields(cls) -> tuple[Field, ...]: +def dataclass_fields(cls: type) -> tuple[Field, ...]: """ Cache the `dataclasses.fields()` call for each class, as overall that ends up around 5x faster than making a fresh call each time. @@ -194,23 +190,23 @@ def dataclass_fields(cls) -> tuple[Field, ...]: """ -def dataclass_init_fields(cls) -> tuple[Field, ...]: +def dataclass_init_fields(cls: type) -> tuple[Field, ...]: """Get only the dataclass fields that would be passed into the constructor.""" -def dataclass_field_names(cls) -> tuple[str, ...]: +def dataclass_field_names(cls: type) -> tuple[str, ...]: """Get the names of all dataclass fields""" -def dataclass_field_to_default(cls) -> dict[str, Any]: +def dataclass_field_to_default(cls: type) -> dict[str, Any]: """Get default values for the (optional) dataclass fields.""" -def is_builtin_class(cls): +def is_builtin_class(cls: type) -> bool: """Check if a class is a builtin in Python.""" -def is_builtin(o: Any): +def is_builtin(o: Any) -> bool: """Check if an object/singleton/class is a builtin in Python.""" @@ -227,7 +223,7 @@ def get_class_name(class_or_instance) -> str: """Return the fully qualified name of a class.""" -def get_outer_class_name(inner_cls, default=None, raise_=True): +def get_outer_class_name(inner_cls, default=None, raise_: bool = True) -> str: """ Attempt to return the fully qualified name of the outer (enclosing) class, given a reference to the inner class. @@ -239,11 +235,11 @@ def get_outer_class_name(inner_cls, default=None, raise_=True): """ -def get_class(obj): +def get_class(obj: Any) -> type: """Get the class for an object `obj`""" -def is_subclass(obj, base_cls: type) -> bool: +def is_subclass(obj: Any, base_cls: type) -> bool: """Check if `obj` is a sub-class of `base_cls`""" diff --git a/dataclass_wizard/loaders.py b/dataclass_wizard/loaders.py index 13c79d1b..cd82b135 100644 --- a/dataclass_wizard/loaders.py +++ b/dataclass_wizard/loaders.py @@ -643,7 +643,7 @@ def load_func_for_dataclass( # Fix for using `auto_assign_tags` and `raise_on_unknown_json_key` together # See https://github.com/rnag/dataclass-wizard/issues/137 has_tag_assigned = meta.tag is not None - if has_tag_assigned: + if has_tag_assigned and meta.tag_key not in field_to_parser: json_to_field[meta.tag_key] = ExplicitNull _locals = { diff --git a/tests/unit/test_load.py b/tests/unit/test_load.py index a6a510ac..6ab44d53 100644 --- a/tests/unit/test_load.py +++ b/tests/unit/test_load.py @@ -2441,3 +2441,37 @@ class Example(JSONWizard): # Attempt to serialize an instance, which should raise the error. Example(my_field=3).to_dict() + + +def test_dataclass_in_union_when_tag_key_is_field(): + """ + Test case for dataclasses in `Union` when the `Meta.tag_key` is a dataclass field. + """ + @dataclass + class DataType(JSONWizard): + id: int + type: str + + @dataclass + class XML(DataType): + class _(JSONWizard.Meta): + tag = "xml" + + field_type_1: str + + @dataclass + class HTML(DataType): + class _(JSONWizard.Meta): + tag = "html" + + field_type_2: str + + @dataclass + class Result(JSONWizard): + class _(JSONWizard.Meta): + tag_key = "type" + + data: Union[XML, HTML] + + t1 = Result.from_dict({"data": {"id": 1, "type": "xml", "field_type_1": "value"}}) + assert t1 == Result(data=XML(id=1, type='xml', field_type_1='value')) From 17b5749e6ebc931d98c1d7ab5b945794e86641c2 Mon Sep 17 00:00:00 2001 From: Ritvik Nag Date: Mon, 25 Nov 2024 22:27:31 -0500 Subject: [PATCH 2/3] Fix: dataclasses in `Union` when `Meta.tag_key` is a dataclass field --- dataclass_wizard/dumpers.py | 2 ++ dataclass_wizard/loaders.py | 23 +++++++++++++++++++---- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/dataclass_wizard/dumpers.py b/dataclass_wizard/dumpers.py index a5bda8d8..ee6ba3c1 100644 --- a/dataclass_wizard/dumpers.py +++ b/dataclass_wizard/dumpers.py @@ -431,6 +431,8 @@ def dump_func_for_dataclass(cls: Type[T], show_deprecation_warning(_pre_dict, reason) _locals['__pre_dict__'] = _pre_dict + + # Call the optional hook that runs before we process the dataclass fn_gen.add_line('__pre_dict__(o)') # Initialize result list to hold field mappings diff --git a/dataclass_wizard/loaders.py b/dataclass_wizard/loaders.py index cd82b135..39111fea 100644 --- a/dataclass_wizard/loaders.py +++ b/dataclass_wizard/loaders.py @@ -643,7 +643,11 @@ def load_func_for_dataclass( # Fix for using `auto_assign_tags` and `raise_on_unknown_json_key` together # See https://github.com/rnag/dataclass-wizard/issues/137 has_tag_assigned = meta.tag is not None - if has_tag_assigned and meta.tag_key not in field_to_parser: + if (has_tag_assigned and + # Ensure `tag_key` isn't a dataclass field before assigning an + # `ExplicitNull`, as assigning it directly can cause issues. + # See https://github.com/rnag/dataclass-wizard/issues/148 + meta.tag_key not in field_to_parser): json_to_field[meta.tag_key] = ExplicitNull _locals = { @@ -743,6 +747,7 @@ def load_func_for_dataclass( fn_gen.add_line("field = json_to_field[json_key] = ExplicitNull") fn_gen.add_line("LOG.warning('JSON field %r missing from dataclass schema, " "class=%r, parsed field=%r',json_key,cls,py_field)") + # Raise an error here (if needed) if meta.raise_on_unknown_json_key: _globals['UnknownJSONKey'] = UnknownJSONKey @@ -759,6 +764,11 @@ def load_func_for_dataclass( with fn_gen.except_(ParseError, 'e'): # We run into a parsing error while loading the field value; # Add additional info on the Exception object before re-raising it. + # + # First confirm these values are not already set by an + # inner dataclass. If so, it likely makes it easier to + # debug the cause. Note that this should already be + # handled by the `setter` methods. fn_gen.add_line("e.class_name, e.field_name, e.json_object = cls, field, o") fn_gen.add_line("raise") @@ -772,9 +782,11 @@ def load_func_for_dataclass( fn_gen.add_line(line) with fn_gen.except_(TypeError): - # If the object `o` is None, then raise an error with the relevant info included. + # If the object `o` is None, then raise an error with + # the relevant info included. with fn_gen.if_('o is None'): fn_gen.add_line("raise MissingData(cls) from None") + # Check if the object `o` is some other type than what we expect - # for example, we could be passed in a `list` type instead. with fn_gen.if_('not isinstance(o, dict)'): @@ -784,8 +796,6 @@ def load_func_for_dataclass( # Else, just re-raise the error. fn_gen.add_line("raise") - # Now pass the arguments to the constructor method, and return the new dataclass instance. - # If there are any missing fields, we raise them here. if has_catch_all: if catch_all_field.endswith('?'): # Default value with fn_gen.if_('catch_all'): @@ -793,8 +803,13 @@ def load_func_for_dataclass( else: fn_gen.add_line(f'init_kwargs[{catch_all_field!r}] = catch_all') + # Now pass the arguments to the constructor method, and return + # the new dataclass instance. If there are any missing fields, + # we raise them here. + with fn_gen.try_(): fn_gen.add_line("return cls(**init_kwargs)") + with fn_gen.except_(TypeError, 'e'): fn_gen.add_line("raise MissingFields(e, o, cls, init_kwargs, cls_fields) from None") From 9c42fdfe33197bb6c4139b63fc7cab2524b0365b Mon Sep 17 00:00:00 2001 From: Ritvik Nag Date: Mon, 25 Nov 2024 22:30:27 -0500 Subject: [PATCH 3/3] Update HISTORY.rst --- HISTORY.rst | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/HISTORY.rst b/HISTORY.rst index 2ca5c861..eb96cb8b 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -2,6 +2,14 @@ History ======= +0.30.1 (2024-11-25) +------------------- + +**Bugfixes** + +* Resolved inconsistent behavior with dataclasses in ``Union`` when ``Meta`` :attr:`tag_key` + is also defined as a dataclass field (:issue:`148`). + 0.30.0 (2024-11-25) -------------------