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

Use simplejson library instead of json #18

Merged
merged 1 commit into from Feb 15, 2017

Conversation

daniel-vera-rodriguez
Copy link
Contributor

We need to serialize a Decimal object in the json object.

For this reason, only changing simplejson library instead of json, we can do it.

@jespino
Copy link
Owner

jespino commented Feb 14, 2017

Hi Daniel,

I'm open to change to simplejson but allowing to use builtin json library, something like

try:
    import simplejson as json
except ImportError:
    import json

And removing the requirement in the installation.

Anyway, you can overwrite the JSONEncoder to allow to encode Decimals without the need of simplejson (you can see an example here: http://stackoverflow.com/questions/1960516/python-json-serialize-a-decimal-object)

@niwinz
Copy link
Collaborator

niwinz commented Feb 14, 2017

Adding support for decimal types can be done with native json module without adding new dependency I think...

@coveralls
Copy link

coveralls commented Feb 15, 2017

Coverage Status

Coverage increased (+0.1%) to 64.69% when pulling f6699cc on daniel-vera-rguez:master into f6ab7ce on jespino:master.

@jespino jespino merged commit dfb6d28 into jespino:master Feb 15, 2017
@jespino
Copy link
Owner

jespino commented Feb 15, 2017

Thanks! ✨

@daniel-vera-rodriguez
Copy link
Contributor Author

daniel-vera-rodriguez commented Feb 15, 2017

Hi guys!

I did the change that @jespino proposed. I had been already seen the link of Stack Overflow and, in fact, in second answer I saw the use of simplejson. I thought that that solution could be less invasive.

But, thank you very much both!

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.

None yet

4 participants