Fix for marshmallow 3.0.0rc7 #29
Conversation
svidela
commented
Jun 19, 2019
•
svidela
commented
- fix needed for marshmallow >= 3.0.0rc7
- fix tests for marshmallow >= 3.0.0
- avoid deprecation warnings
- adds tox environments for python 3.7, marshmallow<3.0.0 and marshmallow>=3.0.0rc7
|
Just wanted to provide the same compatibility fix 👍 |
| @@ -38,7 +38,7 @@ def __init__( | |||
| self.enum = enum | |||
| self.by_value = by_value | |||
|
|
|||
| if error and any(old in error for old in ('{name', '{value', '{choices')): | |||
| if error and any(old in error for old in ('name}', 'value}', 'choices}')): | |||
There was a problem hiding this comment.
Why this change? These are library specific interpolations and not dependent on marshmallow.
There was a problem hiding this comment.
I changed it because as it was before I would always get the warning. Even when using the not deprecated interpolations. But, I just noticed that the pytest version should be >= 3.8.0, otherwise I'm not getting any warnings. Could you confirm this?
There was a problem hiding this comment.
I'll check it out. The issue with checking for the } at the end is there could be formatting directives in between the the target and the end of the interpolation.
| @@ -51,7 +51,7 @@ def __init__( | |||
| if load_by is None: | |||
| load_by = LoadDumpOptions.value if by_value else LoadDumpOptions.name | |||
|
|
|||
| if load_by not in LoadDumpOptions: | |||
| if not isinstance(load_by, Enum) or load_by not in LoadDumpOptions: | |||
There was a problem hiding this comment.
Should probably be not isinstance(load_by, LoadDumpOptions) but the not in check should be good enough, what's the reasoning behind this?
There was a problem hiding this comment.
The aim was to avoid this warning (shown with pytest >= 3.8.0):
DeprecationWarning: using non-Enums in containment checks will raise TypeError in Python 3.8
if load_by not in LoadDumpOptions:There was a problem hiding this comment.
Ah, didn't realize that was a change coming. Carry on. 👌
| @@ -75,13 +77,19 @@ class EnumSchema(Schema): | |||
|
|
|||
| def test_enum_field_load(self): | |||
| serializer = self.EnumSchema() | |||
| data = serializer.load({'enum': 'one', 'none': None}).data | |||
|
|
|||
| data = serializer.load({'enum': 'one'}) | |||
There was a problem hiding this comment.
Would rather either:
- Drop support for Marshmallow 2 and ergo not need this check (would require a major version bump)
- Write separate tests that use
pytest.skipifto avoid introducing versioning checks in the tests
There was a problem hiding this comment.
Hi! should I move forward with option 1?
|
any chance we can get this merged? would love to upgrade to marshmallow v3 - seems like this is our only blocker. thanks for all the work! |
|
I'll do one more review and merge if I don't see anything that's serious when I get home from work today. |
|
Didn't forget to smash that approve and merge button tonight. |
|
v1.5.1 is now live (1.5.1 because pypi added the strict ReST requirement for readmes since last release 😬 ) |
