Python 3 support

[felipou]

I'd like to implement Python 3 support for this package. It's one of the last packages that do not support it here: http://py3readiness.org/

I've been studying Python C extensions and this package's code, and I believe this is doable.

The hardest and most important part is to define how the two functions should work in Python 3. This is what I thought so far:

• JSON_encode: as of now, this function returns a Python 2 str object. Since this is the equivalent of the bytes type in Python 3, I thinks it would be better if this function returned a Python 3 str (equivalent to Python 2 unicode) by default, and we add an argument that makes it return a bytes object (the current behavior).
• JSON_decode: basically I think that the argument all_unicode should have it's default value changed to True in Python 3.

I'm still getting used to Python 3 myself, and I haven't ported much code from Python 2 to 3 yet, so I'm not sure if these are the best options, but I believe they are a good enough first step towards porting, and then we can find someone with more expertise to help decide what would be the best choices for Python 3.

Anyway, thanks a lot for this great package, and I hope you will consider my offer favorably! :)

[danpascu]

I'd like to implement Python 3 support for this package. It's one of the last packages that do not support it here: http://py3readiness.org/

The hardest and most important part is to define how the two functions should work in Python 3. This is what I thought so far:

JSON_encode: as of now, this function returns a Python 2 str object. Since this is the equivalent of the bytes type in Python 3, I thinks it would be better if this function returned a Python 3 str (equivalent to Python 2 unicode) by default, and we add an argument that makes it return a bytes object (the current behavior).

I do not think this new argument is absolutely necessary. If it always returns a Python3 str, bytes can be obtained with: bytes(cjson.encode(data), 'utf8')
Maybe for convenience it could be useful to have this argument, to avoid extra manual conversions like this.

JSON_decode: basically I think that the argument all_unicode should have it's default value changed to True in Python 3.

I think the argument should disappear completely and the code should behave as if all_unicode is True. I see no point for the decode function to ever return bytes in Python3.

[felipou]

I'll implement JSON_encode without the new argument and always returning str then. Afterwards I can add the conversion if we decide it's better to have it.

And I'll implement JSON_decode always returning str, without the parameter.

I'm hoping to create a pull request soon (possibly next week). :)

[felipou]

I did an initial implementation on a branch in my fork:
https://github.com/felipou/python-cjson/tree/python3support

I started by implementing support for both versions (2 and 3), but afterwards I gave up and decided to just try to make it work on Python 3 first. And voilà, I succeeded, all tests have passed! :)

I'm not 100% confident that everything is as it should be yet, but at least we have something that works now. I'll try to implement support for both, and then maybe we can call for help to review and test everything.

[felipou]

My references for implementing were basically these:
https://docs.python.org/3/howto/cporting.html
https://docs.python.org/3/c-api/

I also got some ideas from around the internet (googling around), and I came across this code (from the numpy project, which is awesome by the way):
https://github.com/numpy/numpy/blob/master/numpy/core/include/numpy/npy_3kcompat.h

Maybe I'll create something similar (although much simpler and smaller), just for the functions we need. I think there won't be many of these, so maybe we won't need a separate file like them.

[felipou]

I replaced PyString_AS_STRING with PyUnicode_AS_UNICODE, but the latter is now deprecated (https://docs.python.org/3/c-api/unicode.html#c.PyUnicode_AS_UNICODE). I'll try to reimplement the encode_unicode function using PyUnicode_WRITE instead, but I'm worried there could be some performance implications.

[collinanderson]

For anyone wanting to see the latest diff, it's here: master...felipou:python3support

[jayvdb]

@felipou , any progress?

It is fairly trivial to integrate your patch for Python 3 support. I was able to do it with some funky RPM .spec logic, so I have tests against Python 3.4, 3.6 and 3.7 happening at https://build.opensuse.org/package/show/home:jayvdb:django/python-python-cjson - all green.

It would be a shame to not include it here and get it released, even if it isnt ideal. It would be possible, and perhaps maybe even appropriate, for this package to have different source .c for Python 3. setup.py can easily choose different .c for each platform, like I did in the RPM .spec

Your test logic changes can easily be changed to be support different Python versions dynamically.