Removing use_enum & use_datetime from from_dict() in favor of auto detection? about mashumaro HOT 6 CLOSED

I'm closing this issue since use_enum and use_datetime were removed in version 3.0.

from mashumaro.

I added use_enum, use_datetime and use_bytes flags because packages for json, msgpack and yaml could parse data right into enum, datetime or bytes instances instead of bare strings. It's just standard types for some decoder libraries.

Also I should notice that calling isinstance function at runtime goes against performance. That's why it's not a good idea.

from mashumaro.

@ydylla The goal of mashumaro's design is to maintain the best performance in most cases. But now it's possible to change the generated code with configuration such as field metadata options. I think the best way here is to add a new flag that would add isinstance call. At the moment we could add this flag inside the metadata options of each field. If you could add a new option, there would be no problems for me to accept such a pull request :) Also I'm planning to add a global config inside a dataclass that could change default behaviour in one place. It would be nice to have a choice of where to change the parameters.

from mashumaro.

Also keep in mind that simple isinstance could not be enough in some cases like List[Dict[str, List[float]]] fields. For real validation we need something else.

from mashumaro.

It's just standard types for some decoder libraries

Thats also why I noticed it, I tried to use the form_dict function on Record objects returned by asyncpg, which supports types like UUID, datetime & IPv4Address.

Right now I can not use from_dict because it breaks on UUID and for the datetimes I would have to add use_datetime everywhere. And then remember to not use these code paths for dicts that come from other sources where the datetimes could be strings.

Adding an options flag (use_uuid, ...) for each type where the constructor can not accept an instance of itself feels not right. Thats why I wanted to add an isinstance check.

But you are right a global isinstance check (at the top of from_dict) does not work for nested container types.

I think a compromise would be possible where the isinstance check happens directly before the constructor of each special type:

elif origin_type is uuid.UUID:
            return overridden or f"{value_name} if isinstance({value_name}, {type_name(origin_type)}) else uuid.UUID({value_name})"

This would allow the dict to contain uuids without adding a new extra option. For Path and IPv4Address the instance check is also faster than using the constructor on existing instances.
For datetimes it would be around 5% slower but in my opinion the gained flexibility and removal of use_datetime would be more important. But we probably can not remove use_dateime for compatibility reasons.

I will prepare a pull request so you can look at it with all proposed changes.

import timeit
from dataclasses import dataclass
from datetime import datetime
from ipaddress import IPv4Address
from pathlib import Path
import uuid
from uuid import UUID

from mashumaro import DataClassDictMixin


@dataclass
class A(DataClassDictMixin):
    ip: IPv4Address

@dataclass
class B(DataClassDictMixin):
    d: datetime

@dataclass
class C(DataClassDictMixin):
    p: Path

@dataclass
class D(DataClassDictMixin):
    u: UUID

if __name__ == '__main__':
    # 5.283844800000001 no instance check
    # 2.4646422999999995 with instance check
    print(min(timeit.repeat("""A.from_dict({"ip": IPv4Address("127.0.0.1")})""", globals=globals())))

    # 0.8063721000000004 no instance check
    # 0.8445386000000004 with instance check, instead of use_datetime
    print(min(timeit.repeat("""B.from_dict({"d": datetime.now()}, use_datetime=True)""", globals=globals())))

    # 5.1607049 no instance check
    # 2.6398259 with instance check
    print(min(timeit.repeat("""C.from_dict({"p": Path("/a")})""", globals=globals())))

    # 1.9809536999999997 with instance check
    # 1.9536457 no instance check, fake use_uuid=True
    # 4.0846588 with uuid.UUID(str(value))
    print(min(timeit.repeat("""D.from_dict({"u": uuid.uuid4()})""", globals=globals())))

from mashumaro.

Hi @ydylla

JFYI, since I recently implemented the dialects feature, I’m planning to remove use_bytes, use_enum, use_datetime parameters. These three parameters were added due to differences in third-party libraries and can now be replaced by dialects.

from mashumaro.