Skip to content
This repository has been archived by the owner on Apr 18, 2018. It is now read-only.

Depend on simplejson for Python < 2.7 #47

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

patricklucas
Copy link
Contributor

This adds a dependency on simplejson for Python < 2.7 and moves the
stanza to import the appropriate json module into pyleus.compat.

Closes #46

This adds a dependency on simplejson for Python < 2.7 and moves the
stanza to import the appropriate json module into pyleus.compat.

Closes Yelp#46
if python_version < (2, 7):
import simplejson as json
else:
import json
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this way we are preventing the use of simplejson for python 2.7 and 3.x, right? I know that a try/catch import here misses the point of this change (even though it would still simplify all other modules), but I feel we are losing something that might be useful (e.g. from my measurements, simplejson seemed to achieve lower latencies than json while processing tuples).

Nevertheless, I don't want to block the pull request for such an issue, since we ship messagepack as default encoding format and simplejson has problems too (https://code.google.com/p/simplejson/issues/detail?id=40). I just wanted to point it out.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you do that test on 2.7? My understanding was that the real performance benefit was on 2.6, but that come 2.7 it was negligible since the relevant improvements had been mainlined.

@poros
Copy link
Contributor

poros commented Oct 21, 2014

Mmm, why Travis' build status is missing on this request?

@patricklucas
Copy link
Contributor Author

Because Travis misses some PRs for some reason. :/

I'm going to modulate this branch to "just" move the try/except into compat.py for now.

@sontek
Copy link

sontek commented Feb 3, 2015

@poros This has mereg conflicts, can you fix it up?

@poros
Copy link
Contributor

poros commented Feb 4, 2015

My concerns about json vs simplejson performances in Python 2.7 are actually fading out. If @plucas is up for it, we may want to unblock this pull request and go ahead with the merge.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add dependency on simplejson for Python <2.7
3 participants