search

crs4 / hl7apy

Python library to parse, create and handle HL7 v2 messages.
http://crs4.github.io/hl7apy/
MIT License
231 stars 92 forks source link

Make compatible with Python 3 #23

Closed lucaswiman closed 7 years ago

lucaswiman commented 7 years ago

@svituz: Similar to @eheinz's work in #19. This PR adds compatibility with Python 3, without adding any external dependencies like six.

I tried to make the commits pretty atomic and homogeneous. The only difficult change was the encoding changes in mllp.py, which I'll comment on below.

Testing

I verified that the tests pass on python 2.7, 3.4 and 3.5 on OS X. I updated the travis.yml file so it should also run on those versions.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.1%) to 92.517% when pulling 699e54f3c22f9e0a07d1731000454a9e85145e06 on lucaswiman:py3k-take-2 into 934c5d483e0489052fb3e4263435aeb2d785f4c4 on crs4:develop.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.1%) to 92.517% when pulling 699e54f3c22f9e0a07d1731000454a9e85145e06 on lucaswiman:py3k-take-2 into 934c5d483e0489052fb3e4263435aeb2d785f4c4 on crs4:develop.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.1%) to 92.511% when pulling 76b55a79d7f61b94f0db81aff862ec9aa756f760 on lucaswiman:py3k-take-2 into 934c5d483e0489052fb3e4263435aeb2d785f4c4 on crs4:develop.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.1%) to 92.554% when pulling 0264e020a7dc3d357d2ed404ba36e40cbed87c5f on lucaswiman:py3k-take-2 into 934c5d483e0489052fb3e4263435aeb2d785f4c4 on crs4:develop.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.1%) to 92.554% when pulling 50ead76f9e03a33570a973a6e1cc2eef3f226143 on lucaswiman:py3k-take-2 into 934c5d483e0489052fb3e4263435aeb2d785f4c4 on crs4:develop.

svituz commented 7 years ago

Hello Lucas, thank you for your good work. Python 3 support is one of the main goal we wanted to accomplish. I commented some lines that I would like you to change before merging your pull request. Also I noticed that you continue your work in your branch so if you want you can update your pull request with new changes. Thanks again, Vittorio

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.1%) to 92.554% when pulling e3769e5b6abaa8be7bd021b20d4ac8e08c345e4c on lucaswiman:py3k-take-2 into 934c5d483e0489052fb3e4263435aeb2d785f4c4 on crs4:develop.

lucaswiman commented 7 years ago

I've addressed the code review comments, other than updating the server.py file to use MLLP, which I don't feel very qualified to do.

Please let me know if there are additional changes you'd like me to do.

svituz commented 7 years ago

Hello Lucas, I merged your pull requests. Regarding the server.py file, that was just a comment about the fact that we need to change the example, I didn't mean you had do that.

lucaswiman commented 7 years ago

Thanks very much @svituz!