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

Assert that Serializer was initialized with data=something in is_valid() #2228

Closed
loic opened this issue Dec 8, 2014 · 8 comments
Closed

Comments

@loic
Copy link
Contributor

loic commented Dec 8, 2014

>>> serializer = SnippetSerializer(snippet)
>>> serializer.is_valid()
False
>>> serializer.errors
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/Users/loic/Dev/django-rest-framework/rest_framework/serializers.py", line 429, in errors
    return ReturnDict(ret, serializer=self)
  File "/Users/loic/Dev/django-rest-framework/rest_framework/utils/serializer_helpers.py", line 13, in __init__
    super(ReturnDict, self).__init__(*args, **kwargs)
  File "/usr/local/Cellar/python/2.7.8_2/Frameworks/Python.framework/Versions/2.7/lib/python2.7/collections.py", line 52, in __init__
    self.__update(*args, **kwds)
  File "/Users/loic/Dev/rest_tutorial/./.venv/bin/../lib/python2.7/_abcoll.py", line 566, in update
    for key, value in other:
ValueError: too many values to unpack
@vccabral
Copy link
Contributor

vccabral commented Dec 8, 2014

@loic, how are you doing that from the interpreter shell when testing the docs? Are you going through the process of setting up the snippet code? Just wondering to see if you have a github project I can clone to try and recreate this issue.

@xordoquy
Copy link
Collaborator

xordoquy commented Dec 8, 2014

You need to instantiate your serializer with data=snippet, otherwise it is assumed to be an instance, not data.

>>> from snippets.serializers import SnippetSerializer
>>> s = SnippetSerializer({'title': 'something'})
>>> s.is_valid()
False
>>> s.errors
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/Users/xordoquy/.virtualenvs/test/lib/python3.4/site-packages/rest_framework/serializers.py", line 426, in errors
    return ReturnDict(ret, serializer=self)
  File "/Users/xordoquy/.virtualenvs/test/lib/python3.4/site-packages/rest_framework/utils/serializer_helpers.py", line 12, in __init__
    super(ReturnDict, self).__init__(*args, **kwargs)
  File "/Users/xordoquy/.virtualenvs/test/lib/python3.4/collections/__init__.py", line 56, in __init__
    self.__update(*args, **kwds)
  File "/Users/xordoquy/.virtualenvs/test/bin/../lib/python3.4/_collections_abc.py", line 602, in update
    for key, value in other:
ValueError: too many values to unpack (expected 2)
>>> s = SnippetSerializer(data={'title': 'something'})
>>> s.is_valid()
False
>>> s.errors
ReturnDict([('code', ['This field is required.'])])
>>> 

@xordoquy xordoquy closed this as completed Dec 8, 2014
@loic
Copy link
Contributor Author

loic commented Dec 8, 2014

@vccabral I used the model and serializer from the tutorial part 1. Obviously the docs don't recommend to do this, I was just experimenting.

@xordoquy sorry if I wasn't clear, this is an enhancement proposal, not a bug report. I propose that is_valid() raises an error when data is missing.

This was discussed a few days ago on IRC:

loic84: tomchristie: SnippetSerializer(snippet).is_valid() is that supposed to work? or is_valid() should only be called when there is data?
loic84: data as in SnippetSerializer(data=data)
tomchristie: loic84: Really `is_valid()` should raise an assertion error if you call it without having passed `data=...`
tomchristie: It's doesn't yet, tho

@xordoquy xordoquy reopened this Dec 8, 2014
@tomchristie tomchristie added this to the 3.0.1 Release milestone Dec 8, 2014
@vccabral
Copy link
Contributor

vccabral commented Dec 8, 2014

@loic what is the "django-rest-framework" unit test way to do an self.assertRaises?

https://docs.python.org/2/library/unittest.html

@xordoquy
Copy link
Collaborator

xordoquy commented Dec 8, 2014

@vccabral we are using py.test so http://pytest.org/latest/assert.html#assertions-about-expected-exceptions
Please also note that py.test works well with unit test's assert* - not sure we started to clean them.

@vccabral
Copy link
Contributor

vccabral commented Dec 8, 2014

@xordoquy thanks!

@vccabral
Copy link
Contributor

vccabral commented Dec 8, 2014

If I read the issue correctly this might be the fix. @loic @xordoquy any feedback is appreciated.

#2234

@tomchristie
Copy link
Member

Thanks for your work on this! Based on this, and the issues noted in #2276, #2284 I've issued pull request #2291 which superseeds this one. Thanks for getting things rolling!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants