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

Update Decimal field serialization to use fixed-point notation #534

Merged
merged 2 commits into from Sep 25, 2016

Conversation

Projects
None yet
3 participants
@gdub
Contributor

gdub commented Sep 23, 2016

This avoids engineering-notation output, which occurs with default Decimal type string formatting in certain situations.

While Python does not have an issue round-tripping the engineering-notation strings, I imagine the fixed-point notation [1] more compatible with other languages/parsers. FWIW, Django REST framework also uses fixed-point notation string output for Decimals [2].

Old behavior:

>>> fields.Decimal(as_string=True).serialize('x', {'x': Decimal('0.0000000')})
'0E-7'

New behavior:

>>> fields.Decimal(as_string=True).serialize('x', {'x': Decimal('0.0000000')})
'0.0000000'
Updated Decimal field serialization to use fixed-point notation.
This avoids engineering notation output, which occurs with default Decimal
type string formatting in certain situations.  For example:
>>> str(Decimal('0.0000000'))
'0E-7'

@gdub gdub changed the title from Updated Decimal field serialization to use fixed-point notation. to Update Decimal field serialization to use fixed-point notation Sep 23, 2016

"""
user.m1 = '0.00000000100000000'
field = fields.Decimal()

This comment has been minimized.

@maximkulkin

maximkulkin Sep 23, 2016

Contributor

How about caching serialization result instead of calling serialization twice?

result = fields.Decimal().serialize('m1', user)
assert isinstance(result, decimal.Decimal)
assert result == decimal.Decimal(user.m1)

result = fields.Decimal(as_string=True).serialize('m1', user)
assert isinstance(result, basestring)
assert result == user.m1
@maximkulkin

LGTM 👍

Just make sure to squash commits together before you merge it.

@sloria

sloria approved these changes Sep 25, 2016

Looks good. Thanks!

@sloria sloria merged commit b85dac1 into marshmallow-code:dev Sep 25, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment