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

serpy no longer supports None as field values #56

Closed
terite opened this issue Nov 28, 2017 · 1 comment
Closed

serpy no longer supports None as field values #56

terite opened this issue Nov 28, 2017 · 1 comment

Comments

@terite
Copy link

terite commented Nov 28, 2017

As a result of #38 serpy the following things happen when None is returned from a MethodField

  • If required=True (default), serialization throws an exception
  • If required=False, the key and value are not included in the output dictionary.

I'm using MethodField because it's a concrete example. I believe the same thing happens with most (if not all) other fields.

As you might expect, this breaks any use case of serpy that needs to return None.

The django rest framework docs state

Setting this to False also allows the object attribute or dictionary key to be omitted from output when serializing the instance. If the key is not present it will simply not be included in the output representation.

In concrete terms, the following serializer returns the data as seen below

class DrfSerializer(serializers.Serializer):
    value = serializers.IntegerField(required=...)
DRF 3.7.3 required=True required=False
data = {'value': 1} {'value': 1} {'value': 1}
data = {'value': None} {'value': None} {'value': None}
data = {} KeyError(u"Got KeyError when... {}

When using the following serpy 0.2.0 serializer I get the results below

class SerpySerializer(serpy.DictSerializer):
    value = serpy.Field(required=...)
serpy 0.2.0 required=True required=False
data = {'value': 1} {'value': 1} {'value': 1}
data = {'value': None} TypeError('Field {0} is required', 'value') {}
data = {} KeyError('value',) KeyError('value',)

For reference, this is the output from serpy 0.1.1

serpy 0.1.1 required=True required=False
data = {'value': 1} {'value': 1} {'value': 1}
data = {'value': None} {'value': None} {'value': None}
data = {} KeyError('value',) KeyError('value',)

As seen in the tables, PR #38 does the following things

  • breaks cases that previously matched DRF behavior
  • adds no new cases that match DRF behavior
  • breaks any serializers that wanted to return None.

For these reasons I'd like to suggest #38 be reverted.

@clarkduvall
Copy link
Owner

Released this change in version 0.3.0

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

No branches or pull requests

2 participants