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

omit_defaults does not omit tuples and frozensets #652

Closed
jodal opened this issue Mar 9, 2024 · 2 comments · Fixed by #653
Closed

omit_defaults does not omit tuples and frozensets #652

jodal opened this issue Mar 9, 2024 · 2 comments · Fixed by #653

Comments

@jodal
Copy link

jodal commented Mar 9, 2024

Description

When a struct has omit_defaults=True set, any fields defaulting to dict, list, or set is omitted from the serialization, as described in the docs.

However, if one is using tuple instead of list and frozenset instead of set as default values, to make the struct as immutable as possible, those defaults are included in the encoded format.

I think that tuple and frozenset should be handled in the same way as dict, list, and setbyomit_defaultsandrepr_omit_defaults`.

Example

import msgspec


class Backpack(msgspec.Struct, omit_defaults=True):
    a_list: list[str] = msgspec.field(default_factory=list)
    a_tuple: tuple[str, ...] = msgspec.field(default_factory=tuple) 
    a_frozenset: frozenset[str] = msgspec.field(default_factory=frozenset) 


instance = Backpack()
print(msgspec.json.encode(instance))

Output:

b'{"a_tuple":[],"a_frozenset":[]}'

Expected:

b'{}'
@jcrist
Copy link
Owner

jcrist commented Mar 10, 2024

Thanks for opening this - this should be fixed by #653. Note that for these cases specifying the default value by value (rather than via default_factory) already works:

import msgspec


class Backpack(msgspec.Struct, omit_defaults=True):
    a_list: list[str] = []
    a_tuple: tuple[str, ...] = ()
    a_frozenset: frozenset[str] = frozenset()


instance = Backpack()
print(msgspec.json.encode(instance))
#> b'{}'

The reason tuple and frozenset are the special cases here is that they're both immutable types and may always be specified by value. Since there's no functional reason to use a default_factory for these types, we hadn't implemented the logic to check for equivalent defaults based on default_factory for tuple/frozenset.

@jodal
Copy link
Author

jodal commented Mar 10, 2024

Thank you for the very swift fix! Especially useful with the tip on how to achieve this without waiting for a new release :-)

jodal added a commit to jodal/mopidy that referenced this issue Mar 10, 2024
jodal added a commit to jodal/mopidy that referenced this issue Sep 15, 2024
jodal added a commit to jodal/mopidy that referenced this issue Sep 15, 2024
jodal added a commit to jodal/mopidy that referenced this issue Sep 15, 2024
jodal added a commit to jodal/mopidy that referenced this issue Sep 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants