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

Allow enum dict keys #215

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

RunOrVeith
Copy link
Contributor

This pull requests allows enums as dict keys.
Since marshmallow does not support anything but string enums by default (see #213 ),
the marshmallow code for enum dict keys also only works with strings.
I've implemented it so that it works for other stuff (e.g. float, int) when using from_json, to_json,
if that is not desired let me know.

This means you can now do:

@dataclass_json
@dataclass(frozen=True)
class DataWithEnumKeys:
    name: str
    enum_dict: Dict[MyEnum, str]


d_keys = DataWithEnumKeys(name="name1", enum_dict={MyEnum.STR1: "str_test",
                                                   MyEnum.INT1: "int_test"})
d_keys.to_json() # '{"name": "name1", "enum_dict": {"str1": "str_test", "1": "int_test"}}'
DataWithEnumKeys.from_json(d_keys.to_json()) == d_keys  # True

@lidatong

@RunOrVeith
Copy link
Contributor Author

@lidatong

res = type_(value)
# we got the enum value
for enum_member in type_:
# We rely on the user that the enum values are unique
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe this is necessarily true -- enum names are not necessarily one-to-one with values. There is a decorator @unique that enforces it, but the standard library does allow for it

Copy link
Owner

@lidatong lidatong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Appreciate the contribution again! But I think this is functionality should be supplied via config by the user. Specifically, you can specify the decoder for the field. I purposely limited the supported types, with the intention of only supporting the most common use-cases and the user being able to extend the library otherwise.

While I see the motivation / use-case for Enum keys in dicts, I don't think the library code itself should try to do that decoding -- there's a tiny bit of code overhead with each feature that adds up to the library's maintainability. I'm struggling to read through / follow the library code much more compared to before, and I'm feeling quite strongly to not add any more features / additions, and focus on closing issues / performance / refactoring / simplifying.

I do appreciate your work on this library a lot, and let me know if you think differently.

@ole-tidal
Copy link

I would like to have this feature as well. Super handy to have especially when coming from Jackson in java.

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 this pull request may close these issues.

3 participants