Allow propagation of class based discriminator settings to subclasses about mashumaro

[Fatal1ty]

Hi @Sanavesa

Deserialization of a dictionary to an object should be successful through any class in the hierarchy (including abstract classes) as long as the dictionary represents a concrete subclass that implements all abstract methods. This is expected to work because of the specified discriminator field that indicates the concrete subclass type.

Since you mentioned abstract classes in the context of inclusion, I assume you expect a concrete subclass to be used to deserialize any of its subclasses? What would you be expecting in the following case? An error or a Bar instance? And if it's a second option, then how should we be able to validate that the incoming dictionary contains exactly the "foo" type?

from dataclasses import dataclass
from typing import Literal

from mashumaro import DataClassDictMixin
from mashumaro.config import BaseConfig
from mashumaro.types import Discriminator


@dataclass
class Base(DataClassDictMixin):
    class Config(BaseConfig):
        discriminator = Discriminator(field="type", include_subtypes=True)


@dataclass
class Foo(Base):
    type: Literal["foo"] = "foo"


@dataclass
class Bar(Foo):
    type: Literal["bar"] = "bar"


obj = Foo.from_dict({"type": "bar"})  # what to expect here?

[Sanavesa]

In the code example provided, I noticed that type is a Literal. This raises a question: should validation or discrimination take precedence? I lean towards validation (and so does the current implementation), which means an error like mashumaro.exceptions.InvalidFieldValue: Field "type" of type Literal['foo'] in Foo has invalid value 'bar' should occur. However, if type were a str (specifically, ClassVar[str]), I'd then expect the output to be a Bar instance instead.

Regarding class hierarchy and their subtype variants, each class seems to have its own lookup map (__mashumaro_subtype_variants__) including itself (unless it's abstract) and its descendants. Given the hierarchy:

    Base
   /    \
Foo    AbstractFoo
 |          |
Bar     ConcreteBar

The expected lookup maps would be:

• Base.__mashumaro_subtype_variants__ = {"foo": Foo, "bar": Bar, "concrete_bar": ConcreteBar}
• Foo.__mashumaro_subtype_variants__ = {"foo": Foo, "bar": Bar}
• Bar.__mashumaro_subtype_variants__ = {"bar": Bar}
• AbstractFoo.__mashumaro_subtype_variants__ = {"concrete_bar": ConcreteBar}
• ConcreteBar.__mashumaro_subtype_variants__ = {"concrete_bar": ConcreteBar}

In the from_dict() method, it should first identify the discriminator field, check its value in the data dictionary, and then look it up in the __mashumaro_subtype_variants__. If a matching class is found (and all fields are present), it should return data as that class. If not, it should raise a mashumaro.exceptions.SuitableVariantNotFoundError.

What are your thoughts on this approach? Thank you for considering my feedback.

[Fatal1ty]

Sorry for being silent for a long time and thank you for the detailed answer. I like your proposal, however, its implementation may require heavy improvements.

Regarding class hierarchy and their subtype variants, each class seems to have its own lookup map (__mashumaro_subtype_variants__) including itself (unless it's abstract) and its descendants.

Unfortunately, this is not true in the current implementation. When a dataclass has a discriminator in its config, a map __mashumaro_subtype_variants__ is being created only for that dataclass. I dropped a note in README about this limitation:

The only difference is that you can't use include_supertypes=True because it would lead to a recursion error.

I see the following way to resolve this issue:

1. Allow using include_supertypes=True along with include_subtypes=True in class level discriminators. This can be done by generating two methods for the same class — the first one will search for a suitable class in __mashumaro_subtype_variants__ and the second one will be used for deserialization of the current class itself.
2. Add an optional flag to Discriminator that will enable propagation of discriminator settings to the subclasses.

With an existing workaround, I think this issue will have a low priority. However, if you or someone else can handle it, PR welcome.