search

HDFGroup / hdf5-json

Specification and tools for representing HDF5 in JSON
https://hdf5-json.readthedocs.io
Other
72 stars 25 forks source link

Support Python 3 #39

Closed mivade closed 8 years ago

mivade commented 8 years ago

New projects should be using modern versions of Python which makes this library not possible to use at the moment. The main issue I see so far is using the legacy print statement instead of the modern print function. Additional problem areas may arrise, but these should be easily found using an automated testing tool such as tox.

I am willing to help with this, and in fact have started work on this.

jreadey commented 8 years ago

@mivada - are you suggesting converting the project to Python 3 or making it Python 2/3 compatible (using python_future or such)? We have some Python 2 projects that are dependent on hdf5-json, so we wouldn't be able to do the former right now.

h5serv as a top-level project would be able to switch to a pure Python 3 implementation.

If you would like to submit a PR for this, it would be most appreciated!

mivade commented 8 years ago

@jreadey: The optimal case is supporting both, which was my intention. After looking into it, it is not as difficult as I feared. I have fixed all but two test failures so far by using the six compatibility layer (which is already a dependency of h5py anyway).

I also plan to get h5serv working on Python 3 as well. Since it's using Tornado, which has good test coverage and broad Python version support, there is also no reason to make it 3-only.

jreadey commented 8 years ago

@mivade - sounds good. I'll look forward to your PR. The hdf5-json test.py will be good to verify everything is working ok. I'm planning on setting up Travis soon so that we can validate various python flavors with each checkin.

jreadey commented 8 years ago

@mivade - will you have a PR for the Python 3 support ready soon?

mivade commented 8 years ago

It's turned out to be a bit harder than I initially thought. Maybe I can refactor what I have so far and submit a PR as a work in progress to get some input?

The main issue was that sometimes strings are being converted to bytes when they shouldn't be and I was unable to figure out where exactly that was happening. Generally, I think it would be best to follow the advice given here.

jreadey commented 8 years ago

Yes, that would be good. I'd like to make hdf5-json py2/py3 compliant and then move the h5serv project to py3.4.

Did you see this commit: https://github.com/HDFGroup/hdf5-json/commit/ce7d12a18a6148d7db790eb3c95b724599873c8b? This may overlap some of the changes you were looking at.

For the string conversion issue, if you can add TODO comment wherever this comes up, I'll take a look.

jreadey commented 8 years ago

More updates here: PR #46.

I've got all the unit test working now. But I think I need to go back and have ascii values returned as unicode strings. Otherwise the results are not JSON-serializable. (and hence the integ test fail).

jreadey commented 8 years ago

Changes have been made to return strings as unicode in Python3. This enables return dicts to be json-serializable. If a client needs to determine if the data is actually bytes rather than str, it will need to look at the returned 'type' element. E.g. "charSet": "H5T_CSET_ASCII" vs "charSet": "H5T_CSET_UTF8".

Fixes merged into develop branch and Travis tests for Python 2.7, 3.3, 3.4, and 3.5 passing.

mivade commented 8 years ago

Thanks!