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

What changed in 3.15? #268

Closed
gshank opened this issue Dec 4, 2024 · 2 comments
Closed

What changed in 3.15? #268

gshank opened this issue Dec 4, 2024 · 2 comments
Labels
question Just a question

Comments

@gshank
Copy link
Contributor

gshank commented Dec 4, 2024

We had to pin our code to < 3.15 because a number of things broke with the upgrade. I don't actually think (so far) that it's a problem with mashumaro, it actually looks like it mostly fixed bugs. The handful of things that I know broke:

  1. A field defined Union[str, float] actually also allowed ints, but it didn't serialize correctly so we left out the int (which oddly worked <3.15 but broke with 3.15). With 3.15 the Union[int, float, str] works okay.
  2. A bool field with an invalid string value in a dictionary, got converted to True when deserialized with "from_dict" so the validation which was done after the from_dict got lost. I've refactored the code to validate before performing a "from_dict".
  3. A field defined as Optional[List[str], List[SomeObject]] which was set to List[SomeObject] got serialized to List[str]. I reordered this to have the List[str] after the List[SomeObject] but haven't yet verified that it works.

I've looked through the code changes, but it's not clear to me what drove these changes. I'm asking because I want to know if I should be going through our Unions and changing the order to put string things last.

Thanks,
Gerda Shank
dbt Labs

@Fatal1ty
Copy link
Owner

Hello @gshank

  1. A field defined Union[str, float] actually also allowed ints, but it didn't serialize correctly so we left out the int (which oddly worked <3.15 but broke with 3.15). With 3.15 the Union[int, float, str] works okay.

There were no changes in Union serialization in 3.15. They were in 3.13 in this PR. Before 3.13 the order of basic types (int, float, str, bool) in Union variants had impact on serialization. Since 3.13 this test shows no difference in the order:

def test_union_encoding():
for variants in permutations((int, float, str, bool)):
for value in (1, 2.0, 3.1, "4", "5.0", True, False):
encoded = encode(value, Union[variants])
assert value == encoded
assert same_types(value, encoded)

2. A bool field with an invalid string value in a dictionary, got converted to True when deserialized with "from_dict" so the validation which was done after the from_dict got lost. I've refactored the code to validate before performing a "from_dict".

To be fair, there was no such validation. Most likely, you encountered the same problem as this when any value was passed through as is for the expected field of type bool. In 3.15 it was fixed in order to guarantee that the expected bool value will be of type bool in the end by wrapping it with bool(...). However, if you expect bool inside Union, it will be converted only if the input value was a value that does not correspond to all variants and bool is to the left of others. You can see all test cases here:

@pytest.mark.parametrize(
"test_case",
[
# int | str
UnionTestCase(Union[int, str], 1, 1),
UnionTestCase(Union[int, str], "1", "1"),
UnionTestCase(Union[int, str], 1.0, 1),
UnionTestCase(Union[int, str], 1.2, 1),
UnionTestCase(Union[int, str], "a", "a"),
UnionTestCase(Union[int, str], [1, 2], "[1, 2]"),
# str | int
UnionTestCase(Union[str, int], 1, 1),
UnionTestCase(Union[str, int], "1", "1"),
UnionTestCase(Union[str, int], 1.0, "1.0"),
UnionTestCase(Union[str, int], 1.2, "1.2"),
UnionTestCase(Union[str, int], "a", "a"),
UnionTestCase(Union[str, int], [1, 2], "[1, 2]"),
# str | date
UnionTestCase(Union[str, date], "2024-11-12", "2024-11-12"),
UnionTestCase(Union[str, date], "a", "a"),
# date | str
UnionTestCase(Union[date, str], "2024-11-12", date(2024, 11, 12)),
UnionTestCase(Union[date, str], "a", "a"),
# int | float
UnionTestCase(Union[int, float], 1, 1),
UnionTestCase(Union[int, float], 1.0, 1.0),
UnionTestCase(Union[int, float], "1", 1),
UnionTestCase(Union[int, float], "1.0", 1.0),
# float | int
UnionTestCase(Union[float, int], 1, 1),
UnionTestCase(Union[float, int], 1.0, 1.0),
UnionTestCase(Union[float, int], "1", 1.0),
UnionTestCase(Union[float, int], "1.0", 1.0),
# bool | int
UnionTestCase(Union[bool, int], 1, 1),
UnionTestCase(Union[bool, int], 1.0, True),
UnionTestCase(Union[bool, int], True, True),
UnionTestCase(Union[bool, int], "1", True),
UnionTestCase(Union[bool, int], "1.2", True),
UnionTestCase(Union[bool, int], [1, 2], True),
UnionTestCase(Union[bool, int], "a", True),
# int | bool
UnionTestCase(Union[int, bool], 1, 1),
UnionTestCase(Union[int, bool], 1.0, 1),
UnionTestCase(Union[int, bool], True, True),
UnionTestCase(Union[int, bool], "1", 1),
UnionTestCase(Union[int, bool], "1.2", True),
UnionTestCase(Union[int, bool], [1, 2], True),
UnionTestCase(Union[int, bool], "a", True),
# dict[int, int] | list[int]
UnionTestCase(Union[Dict[int, int], List[int]], {1: 2}, {1: 2}),
UnionTestCase(Union[Dict[int, int], List[int]], {"1": "2"}, {1: 2}),
UnionTestCase(Union[Dict[int, int], List[int]], [1], [1]),
UnionTestCase(Union[Dict[int, int], List[int]], ["1"], [1]),
# str | list[str]
UnionTestCase(Union[str, List[str]], ["a"], ["a"]),
UnionTestCase(Union[str, List[str]], "abc", "abc"),
UnionTestCase(Union[str, List[str]], 1, "1"),
UnionTestCase(Union[str, List[str]], [1, 2], ["1", "2"]),
# int | float | None
UnionTestCase(Union[int, float, None], None, None),
UnionTestCase(Union[int, float, None], 1, 1),
UnionTestCase(Union[int, float, None], 1.2, 1.2),
UnionTestCase(Union[int, float, None], "1", 1),
UnionTestCase(Union[int, float, None], "1.2", 1.2),
UnionTestCase(Union[int, float, None], "a", None),
# None | int | float
UnionTestCase(Union[None, int, float], None, None),
UnionTestCase(Union[None, int, float], 1, 1),
UnionTestCase(Union[None, int, float], 1.2, 1.2),
UnionTestCase(Union[None, int, float], "1", None),
UnionTestCase(Union[None, int, float], "1.2", None),
UnionTestCase(Union[None, int, float], "a", None),
# int | None | float
UnionTestCase(Union[int, None, float], None, None),
UnionTestCase(Union[int, None, float], 1, 1),
UnionTestCase(Union[int, None, float], 1.2, 1.2),
UnionTestCase(Union[int, None, float], "1", 1),
UnionTestCase(Union[int, None, float], "1.2", None),
UnionTestCase(Union[int, None, float], "a", None),
],
)
def test_union_deserialization(test_case):
@dataclass
class DataClass(DataClassDictMixin):
x: test_case.type
instance = DataClass(x=test_case.loaded)
loaded = DataClass.from_dict({"x": test_case.dumped})
assert loaded == instance
assert same_types(loaded.x, instance.x)

3. A field defined as Optional[List[str], List[SomeObject]] which was set to List[SomeObject] got serialized to List[str]. I reordered this to have the List[str] after the List[SomeObject] but haven't yet verified that it works.

I believe you mean Union[List[str], List[SomeObject]] here because Optional[List[str], List[SomeObject]] is invalid syntax. Again, Union serialization was changed in 3.13 and it only concerned basic types, which List[str] and List[SomeObject are not. Instead of reordering you can write it as List[Union[str, SomeObject]]. This form is shorter and allows any order — str, SomeObject or SomeObject, str.

@Fatal1ty Fatal1ty added the needs information Further information is requested label Dec 17, 2024
@gshank
Copy link
Contributor Author

gshank commented Dec 19, 2024

Yes, you're correct that I mean Union[List[str], List[SomeObject]]. You addressed the issue with 3 in the other ticket I opened. I will try the workaround for that.

Thanks

@Fatal1ty Fatal1ty added question Just a question and removed needs information Further information is requested labels Dec 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Just a question
Projects
None yet
Development

No branches or pull requests

2 participants