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

Decimal objects should respect as_string argument at either the validation or deserialization stages #434

Closed
davidthornton opened this Issue Apr 15, 2016 · 0 comments

Comments

Projects
None yet
1 participant
@davidthornton
Contributor

davidthornton commented Apr 15, 2016

The Decimal object subclasses Number which establishes an as_string constructor parameter in order to serialize values as strings.

While this is a good idea and probably caters to a canonical use-case in serialization, there is the potential for unexpected behaviour in deserialization and/or validation (when using the Decimal object, specifically).

This is because the Python decimal.Decimal() method constructs values from integers, strings, floats, or tuples; and (per https://docs.python.org/2.7/library/decimal.html#quick-start-tutorial):

Construction from an integer or a float performs an exact conversion of the value of that integer or float

From the Python documentation, even with as_string set, the current behaviour is to deserialize by passing directly to decimal.Decimal(), causing a situation like this:

>>> Decimal('3.14')
Decimal('3.14')
>>> Decimal(3.14)
Decimal('3.140000000000000124344978758017532527446746826171875')

This behaviour is at best arcane but at worst unexpected or wrong regardless of whether as_string is set.

I propose that when as_string is set to true, either the _deserialize method implicitly casts the value to a string before passing to decimal.Decimal() or non-string values do not pass validation. Legacy behaviour would be preserved when as_string is false.

I prefer the implicit coercion to a string and can submit a small pull request to implement this. Failing validation is a bit more difficult because you have to differentiate between str and unicode - but I have presented it here as a potential solution to the problematic behaviour for comparison.

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