Accept arguments from JSON encoding method #706

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
@mminer

mminer commented Apr 1, 2013

It's impossible to send arguments to Tornado's json_encode function that one might send to Python's built-in json.dumps. In particular, the latter's default argument accepts a function that can be used to tell the encoder how to handle (for example) datetimes, and I would like to continue doing this while using json_encode. Ditto for json_decode.

This fixes that.

Accept arguments from JSON encoding method.
This passes them onto Python built-in JSON encoding library. Also included the
same fix for Tornado's JSON decoding method.
@bdarnell

This comment has been minimized.

Show comment Hide comment
@bdarnell

bdarnell Apr 1, 2013

Owner

This must be our most commonly-repeated bug report, followed closely by one that is diametrically opposed to it. Some people want to tie json_encode and json_decode more closely to their underlying implementation by exposing all its keyword arguments (#566, #414, #646, #54, #16), while others want it to switch between different implementations (usually for performance reasons; #129, #116).

Now that we've dropped support for python 2.5 and the simplejson fallback, it's a little more acceptable to say that json_encode will always be a wrapper around json.dumps and exposes all its implementation details, but why bother? If you need the particulars of a certain json implementation, just call it directly. Tornado doesn't use json much internally (and where it does you wouldn't have the opportunity to pass in these additional arguments).

Owner

bdarnell commented Apr 1, 2013

This must be our most commonly-repeated bug report, followed closely by one that is diametrically opposed to it. Some people want to tie json_encode and json_decode more closely to their underlying implementation by exposing all its keyword arguments (#566, #414, #646, #54, #16), while others want it to switch between different implementations (usually for performance reasons; #129, #116).

Now that we've dropped support for python 2.5 and the simplejson fallback, it's a little more acceptable to say that json_encode will always be a wrapper around json.dumps and exposes all its implementation details, but why bother? If you need the particulars of a certain json implementation, just call it directly. Tornado doesn't use json much internally (and where it does you wouldn't have the opportunity to pass in these additional arguments).

@bdarnell bdarnell closed this Apr 1, 2013

@mminer

This comment has been minimized.

Show comment Hide comment
@mminer

mminer Apr 1, 2013

Fair enough. If json_encodes is simply a wrapper around json.dumps then I see no reason not to pass arguments along — it offers convenience with no discernible downside, reducing the need to duplicate code — but if you want to leave open the possibility of using an alternative JSON library in the future then it makes sense to limit its functionality.

mminer commented Apr 1, 2013

Fair enough. If json_encodes is simply a wrapper around json.dumps then I see no reason not to pass arguments along — it offers convenience with no discernible downside, reducing the need to duplicate code — but if you want to leave open the possibility of using an alternative JSON library in the future then it makes sense to limit its functionality.

@vmarkovtsev

This comment has been minimized.

Show comment Hide comment
@vmarkovtsev

vmarkovtsev Nov 13, 2014

I am another guy who tried to add **kwargs and read the comment above. I am sure not the last one. Just curious: if so many people are unsatisfied with this function, isn't it a sign to do something with it? Wouldn't it be better to remove it? I see the following approaches:

  • Deprecate json_encode, add json_escape which escapes the resulting JSON string (ensures it is escaped).
  • Deprecate json_encode, add json_encode_ex(value, method) to return method(value).replace(...)

BTW, I needed sort_keys set to True to get reproducible test results of the web service. I ended up with copy-pasting json_encode, doing my edits and setting tornado.escape.json_encode to it.

I am another guy who tried to add **kwargs and read the comment above. I am sure not the last one. Just curious: if so many people are unsatisfied with this function, isn't it a sign to do something with it? Wouldn't it be better to remove it? I see the following approaches:

  • Deprecate json_encode, add json_escape which escapes the resulting JSON string (ensures it is escaped).
  • Deprecate json_encode, add json_encode_ex(value, method) to return method(value).replace(...)

BTW, I needed sort_keys set to True to get reproducible test results of the web service. I ended up with copy-pasting json_encode, doing my edits and setting tornado.escape.json_encode to it.

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