Closed estebistec closed 8 years ago
You never updated the requirements.txt
, so none of the tests were able to run.
derp, updated. I was working out of setup.py when I tested the upgrade.
Yes, now exactly the one test that I expect fails.
Any news on this ?
@vperron Some of this is supported in REST framework these days (ListField
, DictField
)
Hi @tomchristie , actually I was interested in ListOrItemField for returning data back from ElasticSearch, which transparently handles collections or single objects (https://www.elastic.co/guide/en/elasticsearch/reference/current/array.html)
I must admit I have no clue how to fix the ValidationError just yet. Did anyone find something about that ?
Here is the fix I think I'll be using:
def to_internal_value(self, data):
if isinstance(data, list):
return self.list_field.to_internal_value(data)
# Force field validation. Not necessary on the list_field since DRF calls it recursively.
self.item_field.run_validation(data)
return self.item_field.to_internal_value(data)
Does it seem valid enough ?
It might be. Let me have a look at the fix tonight. On Wed, Feb 24, 2016 at 7:08 AM Victor Perron notifications@github.com wrote:
Here is the fix I think I'll be using:
def to_internal_value(self, data): if isinstance(data, list): return self.list_field.to_internal_value(data) # Force field validation. Not necessary on the list_field since DRF calls it recursively. self.item_field.run_validation(data) return self.item_field.to_internal_value(data)
Does it seem valid enough ?
— Reply to this email directly or view it on GitHub https://github.com/estebistec/drf-compound-fields/pull/27#issuecomment-188296900 .
Thanks @vperron, looks like that does it. I think when I encountered this error it didn't site right with me that this field should be different from others, but it seems the item case is always relying on a container context to trigger validation.
Please test this out. Let me know if it works for you and I'll cut the release for this. If it doesn't, please supply any more test cases we need to consider fixing first.
Thanks again!
Hrm... I ran the tests locally on py 3.5.1, travis is failing on 3.5.0. I'll look at this more tomorrow. It's passing on 2.7 / 3.4.
Hi @estebistec , I had a look and clearly the failure on 3.5.0 is out of my depths. Dunno if that is an issue with Python itself or not. Do DRF tests pass on 3.5.0 ?
and by tomorrow I mean "this weekend"... it's been a packed week. Looking today or tomorrow.
so rest framework appears to only be tested through python 3.4. Let's send them a quick pull to add 3.5 and see what happens...
nevermind, their travis environments are just named that way. it appears they only test with 3.5.0 :/
there we go. found a diff in the pytest version being used. Looked like that fixed it.
Anything else before I merge and release?
Nope, seems good !
Will release tonight (probably not for a few more hours).
On Mon, Feb 29, 2016 at 3:20 AM Victor Perron notifications@github.com wrote:
Nope, seems good !
— Reply to this email directly or view it on GitHub https://github.com/estebistec/drf-compound-fields/pull/27#issuecomment-190162353 .
Here we go. Lots of code gets deleted (always good). The remaining types are so little that perhaps we can convince of their inclusion in rest-framework. CC: @tomchristie
I've kept the tests of list/dict fields to show that the DRF composite types meet the definition that users of this library had relied on.
There is one remaining failing test:
Whereas the test of a list with items of the wrong length does get the expected validation error. This is the case for any child types that I happened to try. For whatever reason, the validation only kicked in when the child of a ListField. This also gets no validation error: