Skip to content
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

Add coerce_to_json function #94

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

wcooley
Copy link
Contributor

@wcooley wcooley commented Sep 21, 2016

This is a function that tries to map data to one of the handful of primitive types supported by JSON. I have included tests using it both for the default parameter of json.dump and with remap.

There is a bit more work to be done but I wanted to first see if there way any interest in it before I put more effort into it. (I am also open to suggestions for the name.)

@mahmoud
Copy link
Owner

mahmoud commented Sep 21, 2016

Absolutely! This is certainly very interesting! I'm curious, can you go into a bit of detail about where and how you've applied this technique?

As for structural changes, a minor point would mostly be to "vendor in" the first and is_scalar implementations from iterutils to comply with the integration architecture.

@wcooley
Copy link
Contributor Author

wcooley commented Sep 21, 2016

I tend to use _set_s for containers and (especially for test fixtures) types.MappingProxyType, the read-only dict proxy, both of which play havoc with JSON. I am also lazy to the point of masochism and will spend hours developing a general-purpose solution rather than do the same thing twice.

For example, I have a client for an HTTP API that updates groups given a list of identifiers to either add or remove, so I build the list of identifiers that should be present (from a source which will have duplicates), request the current identifiers in the group, do set operations for the adds and removes and then throw the sets of adds and removes at my client. The only downside is that instead of passing the data to requests.requests as requests.requests(..., json=data, ...), I have to use

json_data = json.dumps(data, default=coerce_to_json)
requests.post(..., data=json_data, headers=['content-type': 'application/json'], ...)

Alternatively, I could use remap to convert the data, and did at first, but I felt like I figured out how to make it work by stabbing at it until it bled, rather than actually understanding what was going on, which I did not think would be as maintainable.

@wcooley
Copy link
Contributor Author

wcooley commented Sep 21, 2016

For the vendoring-in of first, is_scalar (and consequently is_iterable), would you prefer them to be:

  1. Defined in the module namespace or within the function itself?
  2. Re-named with a leading underscore?
  3. Imported and only used on ImportError, like namedtuple in Feature: Return namedtuples for get_format_args #93?
  4. Copied wholesale with docstring (which might might comparing for future updates easier), copied without the docstring but the function body as-is or minimized (e.g., for is_scalar I'd copy (is_iterable and switch the return values)?

So many questions for so few lines... Guess I should research what's already being done.

@wcooley
Copy link
Contributor Author

wcooley commented Sep 21, 2016

I looked at the existing functions and did not find too many examples other than make_sentinel, so I've done what seems best to me; let me know if this is the preferred approach.

A class having to an explicit conversion method and also being iterable
seems a likely case, so test for conversion methods before attempting
type-guessing.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants