couchdb-python
couchdb-python copied to clipboard
Support TupleFIeld for tuples with fixed types
From [email protected] on February 12, 2011 00:08:09
The attached patch implements support for this, because I needed it in my project (which uses paisley which now uses a copy of mapping.py)
Note: when I run doctests with
nosetests --with-doctest couchdb/mapping.py
I get errors for ListField, TupleField and ViewField.
Without the patch I only get the errors for ViewField.
Am I running the doctests right ? I don't see why adding TupleField should have an effect on the ListField doctest.
Original issue: http://code.google.com/p/couchdb-python/issues/detail?id=165
From [email protected] on February 11, 2011 15:08:48
Sorry, I opened as Defect and should be Enhancement I guess - no idea how to change it though.
From [email protected] on February 11, 2011 16:23:32
sorry, that patch was merged wrong after updating my python-couchdb checkout.
Here's a better patch, with working doctests.
It did bring to light something that looks wrong with the ListField implementation:
- why shouldn't author and comment be returned as unicode in its doctest ?
- why does time get returned as a string instead of a datetime ?
Compare to e.g. the DictField doctest where name and email get returned as unicode too.
The TupleField implementation is using the field's _to_python implementation, which looks like the right thing to do me.
Attachment: mapping-tuple.patch
From djc.ochtman on February 28, 2011 04:13:24
I'm not sure about this one, though I'm leaning towards including it. On the one hand, we can't actually distinguish between tuples and lists from the JSON data, so it's a bit of guess work. On the other hand, it seems like a useful thing to have for heterogeneously typed JSON lists, which are as useful as Python's tuple.
Matt, what do you think?
Labels: -Type-Defect Type-Enhancement
From [email protected] on February 28, 2011 06:18:43
I have no real issue with including a tuple type in the mapping schema. The Python tuple <-> JSON array conversion is not really any different than a Python datetime.date <-> JSON string conversion - they're both just mechanisms to serialise native Python types to JSON.
Quickly scanning the patch, I think it looks generally good. Just a couple of minor points:
- Should the default be [], the empty list? A (None, None, ...) tuple seems slightly better. Or maybe just None, although that would not be consistent with DictField and ListField.
- Should it check len(self.fields) == len(values) to ensure that items are never discarded?
From djc.ochtman on February 28, 2011 07:24:59
Yeah, I think a (None, None, ...) default would be better. Checking the length also sounds like a good safeguard.
From [email protected] on April 09, 2011 10:46:49
Adding new version of patch taking into account your suggestions.
Attachment: ma
From [email protected] on April 10, 2011 06:45:59
Not sure how that happened, sorry!
Attachment: mapping.patch
From [email protected] on May 19, 2011 06:44:07
Hi guys,
could someone please look at this patch ? It's straightforward to merge.
Thomas
From kxepal on May 21, 2011 03:34:33
My own suggestions:
- replace AssertionError's with ValueError
- handle situation then TupleField would be initialized in same way as ListField (that's intuitive error)
- may be raise ValueError for such initializations: TupleField([IntegerField, 1, None, 3, '...', 5]) because it breaks _to_python / _to_json convertations? Or leave constants as is, but I suppose that would be bad solution.
- tests(: