brack3t / Djrill

[INACTIVE/UNMAINTAINED] Djrill is an email backend and new message class for Django users that want to take advantage of the Mandrill transactional email service from MailChimp.
BSD 3-Clause "New" or "Revised" License
319 stars 64 forks source link

Decimal JSON serialization #89

Closed gcaprio closed 9 years ago

gcaprio commented 9 years ago

When I try and include a Decimal value in my merge_vars, I get this exception:

Decimal('0.00') is not JSON serializable

Seems like the default json serializer doesn't support decimals out of the box. You could notice that in JSONDateUTCEncoder and convert it to a string:

if isinstance(obj, Decimal):
      return "%.2f" % obj
return super(JSONDateUTCEncoder, self).default(obj)
medmunds commented 9 years ago

This is true. Djrill uses Python's standard json package, which doesn't (yet*) handle serializing Decimal numbers.

The problem with trying to work around this in Djrill is that we'd have to make an arbitrary choice about the right way to serialize a decimal number, and we'd probably get it wrong for many of the cases where people use decimals.

Your proposal is that Djrill should convert a decimal to a string with two digits after the decimal point (23.20). This might be the right thing to do for (some) currencies, but it'd often be wrong for stock quotes ($5.6250), or temperatures (18.2°), or shipping weights (12.4kg), or currencies that don't commonly use decimals (¥ 2320), or a bunch of other merge data that might come from Decimal variables.

Since only you know what your decimal data represents, it's much better for you to convert it to a string--with the exact format you need--as you build up your merge vars.

(BTW, Djrill serializes dates and datetimes to JSON, because there's a specific format that the Mandrill API expects for options that take dates, like send_at. But you'd still probably want to do your own formatting if you were going to put a date into a merge var.)

__

* There's actually been discussion of handling Decimal serialization in Python's standard json package. But the proposal seems to be to convert the Decimals to JSON numbers (floats), rather than formatted strings -- which wouldn't be what you're looking for.

gcaprio commented 9 years ago

True, my code snippet was just an example, not a full PR. I still think that it's something that should be at least called out in the docs. It's a tricky error to track down, leading you to think it's a big in Djrill itself.