Code

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#16850 closed Bug (fixed)

Testsuite failing on JSON serialization

Reported by: Raphael Hertzog <hertzog@…> Owned by: nobody
Component: Core (Serialization) Version: 1.3
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Trying to build Django 1.3.1 on Debian unstable with python 2.6 and python-simplejson 2.2.0 installed I get two test suite failures.

Without python-simplejson

======================================================================
ERROR: test_serialize_unicode (modeltests.serializers.tests.JsonSerializerTestCase)
Tests that unicode makes the roundtrip intact
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/rhertzog/deb/pkg/TEAMS/build-area/python-django-1.3.1/tests/modeltests/serializers/tests.py", line 172, in test_serialize_unicode
    obj_list = list(serializers.deserialize(self.serializer_name, serial_str))
  File "/home/rhertzog/deb/pkg/TEAMS/build-area/python-django-1.3.1/django/core/serializers/json.py", line 35, in Deserializer
    for obj in PythonDeserializer(simplejson.load(stream), **options):
  File "/home/rhertzog/deb/pkg/TEAMS/build-area/python-django-1.3.1/django/core/serializers/python.py", line 128, in Deserializer
    data[field.name] = field.to_python(field_value)
  File "/home/rhertzog/deb/pkg/TEAMS/build-area/python-django-1.3.1/django/db/models/fields/__init__.py", line 761, in to_python
    return decimal.Decimal(value)
  File "/usr/lib/python2.6/decimal.py", line 649, in __new__
    "First convert the float to a string")
TypeError: Cannot convert float to Decimal.  First convert the float to a string

======================================================================
ERROR: test_json_serializer (regressiontests.serializers_regress.tests.SerializerTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/rhertzog/deb/pkg/TEAMS/build-area/python-django-1.3.1/django/utils/functional.py", line 55, in _curried
    return _curried_func(*(args+moreargs), **dict(kwargs, **morekwargs))
  File "/home/rhertzog/deb/pkg/TEAMS/build-area/python-django-1.3.1/tests/regressiontests/serializers_regress/tests.py", line 373, in serializerTest
    for obj in serializers.deserialize(format, serialized_data):
  File "/home/rhertzog/deb/pkg/TEAMS/build-area/python-django-1.3.1/django/core/serializers/json.py", line 35, in Deserializer
    for obj in PythonDeserializer(simplejson.load(stream), **options):
  File "/home/rhertzog/deb/pkg/TEAMS/build-area/python-django-1.3.1/django/core/serializers/python.py", line 128, in Deserializer
    data[field.name] = field.to_python(field_value)
  File "/home/rhertzog/deb/pkg/TEAMS/build-area/python-django-1.3.1/django/db/models/fields/__init__.py", line 761, in to_python
    return decimal.Decimal(value)
  File "/usr/lib/python2.6/decimal.py", line 649, in __new__
    "First convert the float to a string")
TypeError: Cannot convert float to Decimal.  First convert the float to a string

I also filed https://github.com/simplejson/simplejson/issues/17 because I'm not sure whether Django is improperly using simplejson or whether simplejson has a bug.

Attachments (0)

Change History (7)

comment:1 Changed 3 years ago by anonymous

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

It looks like the problem here is that Django expects that decimal objects will get converted to string (by its default method) instead of a JSON number. This change to the default options was made for usability reasons in simplejson 2.2.0, most people want decimals to get encoded as numbers even though there's a possible loss of precision if decoded on the other end without the right options. This behavior can be turned back off with use_decimal=False on encode. The version embedded in Django also supports this option (default False rather than True), so explicitly specifying this behavior would be compatible with whichever version of dump/dumps ends up getting used.

comment:2 Changed 3 years ago by Raphaël Hertzog <hertzog@…>

Note that the comment above was made by the simplejson author/maintainer (see https://github.com/simplejson/simplejson/issues/17#issuecomment-2105920).

comment:3 Changed 3 years ago by carljm

  • Triage Stage changed from Unreviewed to Accepted

We should just use that use_decimal=False option, then. I don't see a compelling case for changing the dumped output to use numbers instead of strings for decimals, given that it can lose precision in the round-trip.

comment:4 follow-up: Changed 3 years ago by ramiro

There is something strange in the fact that testing things with Python 2.7 (I also use Debian unstable, Python 2.7 has recently entered Sid) and the same simplejson 2.2.0 version (provided by debian python-simplejson DEB package) doesn't show these failures.

Just to be sure, I ran the serializers tests with Python 2.6 in the same system and I see the reported failures.

Maybe Python 2.6 also has something to do with this?

Last edited 3 years ago by ramiro (previous) (diff)

comment:5 in reply to: ↑ 4 Changed 3 years ago by ramiro

Replying to ramiro:

There is something strange in the fact that testing things with Python 2.7 (I also use Debian unstable, Python 2.7 has recently entered Sid) and the same simplejson 2.2.0 version (provided by debian python-simplejson DEB package) doesn't show these failures.

Just to be sure, I ran the serializers tests with Python 2.6 in the same system and I see the reported failures.

Maybe Python 2.6 also has something to do with this?

The reason for this is because additionally the version of Decimal shipped with Python 2.7 has a new Decimal.from_float() method that gets used during decoding from JSON operations instead or raising the error reported with Python 2.6 by the OP. This method can lose precision and neither we nor simplejson can control that.

This seems to be an additional reason to keep using strings to encode Decimal's

comment:6 Changed 3 years ago by ramiro

  • Resolution set to fixed
  • Status changed from new to closed

In [17228]:

Fixed #16850 -- Made sure we always represent Decimal instances with JS strings when serializing.

We force this when calling simplejson dump() to isolate us from changes of
default behavior to encode them with JavaScript numbers instead that could
introduce loss of precision when decoding back from JSON to Python.

Thanks Raphael Hertzog for the report and Bob Ippolito for his help.

comment:7 Changed 3 years ago by ramiro

In [17229]:

Tweaked changes from r17228 to cater for older simplejson versions.

dump() started accepting the use_decimal argument in 2.1.3.

Refs #16850.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.