Python2/3 compatibile (compile and runtime) code #32
Conversation
change exception syntax (for raise and except)
|
THX for the PR: please give me a week or so for the review. Although it is not strictly python3 compatibility, I would suggest to update also the ownet |
|
You're right. I'll do that. |
There was a problem hiding this comment.
At a first glance the changes to the python sources are OK.
However the setup.py files should be modernised and made python3 compatible.
module/ownet/python/setup.pyandmodule/swig/python/setup.py.in: usesetuptoolsinstead of legacydistutilsmodule/ownet/python/setup.py: add relevantinstall_requires = ['six>= X.Y.Z']module/swig/python/setup.py.in: when I try to build under python3 I have this error:
$ make -C module/swig/python
Making all in ow
make[1]: Nothing to be done for `all'.
/usr/local/bin/swig -python -o ow_wrap.c ../ow.i
CFLAGS="-I/usr/local/Cellar/libusb/1.0.22/include/libusb-1.0 " /Users/miccoli/.pyenv/versions/owfs/bin/python setup.py build
Traceback (most recent call last):
File "setup.py", line 93, in <module>
my_extra_link_args = my_extra_link_args + string.split('../../owlib/src/c/.libs/libow.so', ' ')
AttributeError: module 'string' has no attribute 'split'
make[1]: *** [OW.py] Error 1
make: *** [all-recursive] Error 1
|
@miccoli Apparently I've lost track of this PR. Sorry for that. Personally, after a short period of time I gave up using synchronous ownet module and wrote something with equivalent but asynchronous API, which better suits my needs. I am still willing to make owfs python3-compatibile. If I'm not mistaken almost two years have passed and it still lacks python3 support. For that reason it's been recently masked and is scheduled to be completely removed from Gentoo's repo! I can rewrite and test whatever is necessary on top of your changes #26 to make it compile and work correctly on either python3 or python2 and python3. |
|
@marcin1j we usually discourage people from using the "official" owfs python bindings in new projects, since this code is unmaintained and has some serious limitations. The only reason for keeping it alive, is to ease the transition from legacy python2 code to python3, although a rewrite, targeting one of the various python3 libraries available on PyPi, would be in most cases a better choice. This said I would say that it is better to keep python2 compatibility, which will stay around for a while for running legacy code. As you can see I pushed a few commits to your repo, addressing some of my concerns. I think that now only some testing is needed to see if all works as expected. Final touches, like a more clean |
This makes Python ownet module portable. Requires "six" module version 1.12.0 or newer. Signed-off-by: Marcin Jurkowski <marcin1j@gmail.com>
This makes swig Python code compatible with both Python 2 and Python 3. Required to build package with Python bindings enabled. Additionally, simplify "except ... as" syntax. There's no reference to exception object in "except ... as" body so this clause can be reduced to pure "exception" clause. Signed-off-by: Marcin Jurkowski <marcin1j@gmail.com>
marcin1j
force-pushed
the
pr/20190103-python3-compatibility
branch
from
3ac4eb7
to
686f88a
Compare
|
I rebased this PR on top of #26. SWIG wrapper now compiles fine on both py2/3. Note that you need 70027f5 (either cherry-pick 70027f5 or merge this PR with master branch). For testing purposes I rebased my PR on top of c68deb1 but can't do that on github since #26 is still based on 8750e5a. Since this is the third attempt to upgrade python codebase (#24 addressed |
|
@marcin1j tanks for your efforts. I'm a little busy now, but I will for sure merge this and all related PRs. |
The python support only works with python2, so drop it. Notice that there is a PR adding python3 support, but it is not yet merged: owfs/owfs#32 Signed-off-by: Peter Korsgaard <peter@korsgaard.com> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
This makes both ownet and swig (bindings, examples, unit tests) modules Python2/Python3 compatible. Ownet library requires six module (https://pypi.org/project/six/, it's a runtime dependency).
Note that this PR conflicts with python part of #26.