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

MessagePackDecoder fails in dataclass complex case #252

Closed
Future-Outlier opened this issue Oct 3, 2024 · 4 comments
Closed

MessagePackDecoder fails in dataclass complex case #252

Future-Outlier opened this issue Oct 3, 2024 · 4 comments
Assignees
Labels
bug Something isn't working

Comments

@Future-Outlier
Copy link

Future-Outlier commented Oct 3, 2024

mashumaro version: 3.13.1
Python version: 3.12.4
Operating System: macOS Sonoma 14.3

Description

I'm a maintainer of Flyte, and I encountered an issue while using mashumaro.codecs.msgpack's MessagePackDecoder and MessagePackEncoder. Specifically, the MessagePackDecoder fails to handle a certain edge case.

What I Did

from mashumaro.codecs.msgpack import MessagePackDecoder, MessagePackEncoder
from dataclasses import dataclass, field
from typing import Dict, List, Optional, Union
from enum import Enum
import msgpack

def _default_msgpack_decoder(data: bytes) -> Any:
    return msgpack.unpackb(data, raw=False, strict_map_key=False)

@dataclass
class InnerDC:
    max_depth: int = 10
    max_features: str = "sqrt"
    n_estimators: int = 100

@dataclass
class DC:
    grid: Dict[str, List[Optional[InnerDC]]] = field(default_factory=lambda: {
        'all_types': [InnerDC()],
    })

# Encoding works
encoder = MessagePackEncoder(DC)
msgpack_bytes = encoder.encode(DC())

# Decoding works
decoder = MessagePackDecoder(DC, pre_decoder_func=_default_msgpack_decoder)
python_val = decoder.decode(msgpack_bytes)
assert python_val == DC()

# Another test with Enum
class Status(Enum):
    PENDING = "pending"
    APPROVED = "approved"
    REJECTED = "rejected"

@dataclass
class DC:
    grid: Dict[str, List[Optional[Union[int, str, float, bool, Status, InnerDC]]]] = field(default_factory=lambda: {
        'all_types': [InnerDC()],
    })

encoder = MessagePackEncoder(DC)
msgpack_bytes = encoder.encode(DC())

# Decoding fails
decoder = MessagePackDecoder(DC, pre_decoder_func=_default_msgpack_decoder)
python_val = decoder.decode(msgpack_bytes)
print(python_val)
assert python_val == DC()

In both cases, encoding works, but decoding with MessagePackDecoder fails.

@Future-Outlier
Copy link
Author

In summary, this is a super edge case.

@dataclass
class DC:
    grid: Dict[str, List[Optional[Union[int, str, float, bool, Status, InnerDC]]]] = field(default_factory=lambda: {
        'all_types': [InnerDC()],
    })

@Future-Outlier
Copy link
Author

You might be interested in how mashumaro is used in bytes, so let me give you a brief concept.

encode

python val -> msgpack bytes -> protobuf literal

decode

protobuf literal -> msgpack bytes -> python val

How we create codecs? (encoder and decoder)

We will provide python type to encoder or decoder to serialize/deserialize it now, thank you for looking this, and feel free to ping me to collaborate, I'll try to reply it in 1 day <3

@Future-Outlier Future-Outlier changed the title MessagePackDecoder fails in dataclass complext cases MessagePackDecoder fails in dataclass complex cases Oct 3, 2024
@Future-Outlier Future-Outlier changed the title MessagePackDecoder fails in dataclass complex cases MessagePackDecoder fails in dataclass complex case Oct 3, 2024
@Fatal1ty
Copy link
Owner

Fatal1ty commented Nov 11, 2024

Hi @Future-Outlier

Sorry for the late response, I was on sort of vacation from open source.

I'm sure this issue is related to unions with str and other basic types. As I mentioned here, we can expect a solution very soon. I ran your example with the modifications I made and it passed with the expected output at the end:

DC(grid={'all_types': [InnerDC(max_depth=10, max_features='sqrt', n_estimators=100)]})

@Fatal1ty Fatal1ty added the bug Something isn't working label Nov 11, 2024
@Fatal1ty Fatal1ty self-assigned this Nov 11, 2024
@Fatal1ty
Copy link
Owner

The fix is ​​ready, will be in the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants