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

add support for enums #18

Merged
merged 5 commits into from
May 21, 2019
Merged

Conversation

evanfwelch
Copy link
Contributor

marshmallow_enum is part of the marshmallow ecosystem. Accordingly, as a primary user of dataclasses, I would love marshmallow_dataclass to support enums. This PR adds that functionality as well as the requisite doctests.

Pipfile Outdated
@@ -10,6 +10,8 @@ sphinx = "*"
marshmallow = ">=2.0,<3.0"
typing-inspect = "*"
dataclasses = "*"
marshmallow-dataclass = "*"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm this could be a mistaken addition to pipfile, will remove

@lovasoa
Copy link
Owner

lovasoa commented May 16, 2019

In fact, enums are already supported, but only in the marshmallow3 branch (published as a pre-release to pypi). I did not backport the feature, because I thought marshmallow 3 was going to be released soon, and we could merge it into master. Unfortunately, marshmallow 3 has been a pre-release for several months, and it looks like it won't be released anytime soon.

So enum support is very welcome, but it should be a backport of how it is done in the marshmallow3 branch. Or even better, we may have a single branch (and a single release) that works with both marshmallow 2 and 3. Working with the two in parallel induces a lot of duplicated efforts and confusion.

@evanfwelch
Copy link
Contributor Author

Hey @lovasoa that makes sense, I'm sorry I missed the support in the marshmallow 3 branch. I too have been waiting for marshmallow 3.x for quite some time and when it gets released a lot of my problems will go away. I will take a peek at your marshmallow 3 branch and see if I have any ideas on how to backport or write it once for both versions. Thanks

@evanfwelch
Copy link
Contributor Author

Have a look now, I think this is what you had in mind

@lovasoa
Copy link
Owner

lovasoa commented May 20, 2019

Yep, it's perfect ! You can merge master (and especially 23ce55b) into your branch, and we'll be ready to merge. Don't forget to add marshmallow_enum to the extra_requires in setup.py.

@evanfwelch
Copy link
Contributor Author

Thanks for the code review, I will update this branch with those changes (I like the extra_requires approach too).

Evan Welch added 5 commits May 20, 2019 10:22
remove marshmallow dataclass from pipfile

change implementation of the marshmallow enum support
@lovasoa lovasoa merged commit b83a34a into lovasoa:master May 21, 2019
@lovasoa
Copy link
Owner

lovasoa commented May 21, 2019

It's marges; thank you again @evanfwelch !

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.

2 participants