search

OpenLightingProject / ola

The Open Lighting Architecture - The Travel Adaptor for the Lighting Industry
https://www.openlighting.org/ola/
Other
645 stars 204 forks source link

Python 3 Compatibility #1506

Open peternewman opened 6 years ago

peternewman commented 6 years ago

if i try the simple fade example this does not work: stefan:~/ola/python/examples[master]$ python3 ./ola_simple_fade.py Traceback (most recent call last): File "./ola_simple_fade.py", line 69, in wrapper.AddEvent(SHUTDOWN_INTERVAL, wrapper.Stop) File "/home/stefan/ola/python/ola/ClientWrapper.py", line 287, in AddEvent self._ss.AddEvent(time_in_ms, callback) File "/home/stefan/ola/python/ola/ClientWrapper.py", line 215, in AddEvent heapq.heappush(self._events, event) TypeError: unorderable types: _Event() < _Event()

See: https://groups.google.com/d/msg/open-lighting/7TuvU0T1CAo/0hBbXHw8BgAJ https://groups.google.com/d/msg/open-lighting/7TuvU0T1CAo/oVIYBMk8BgAJ

Hopefully whoever takes on that will give the whole system a bit of a shakedown on Python 3 and fix any further remaining bugs.

peternewman commented 5 years ago

See also https://github.com/OpenLightingProject/ola/pull/1514

peternewman commented 5 years ago

And #1537 / #1538

brucelowekamp commented 4 years ago

I ran into this problem b/c my first attempt to use ola was on a mac with homebrew (btw, thanks, having great fun with ola), and mac homebrew builds it against python 3.

I actually considered whether it would be possible to fix the formula to use python2, but it would require a py2 build of protobuf3.6, which given python 2 is going unsupported (https://pythonclock.org/) etc seemed like it might be hard to get approved (though just copying the python code over works, of course)

I have a local build with #1538 applied that resolved the problems, though as I'm deploying on a RPi I forced everything back to py2 after awhile anyway. Regardless +1 for getting the py3 compatibility shipped. Do agree with the comments on #1538 about the need for more tests especially if intention is to ship both py2 and py3 for awhile. I might be able to devote a bit of time to some of that in the next couple of months, if needed.

peternewman commented 4 years ago

I ran into this problem b/c my first attempt to use ola was on a mac with homebrew (btw, thanks, having great fun with ola), and mac homebrew builds it against python 3.

Yeah I think I twigged that recently while digging into the protobuf changes they'd made to our formula. As an actual user of our Homebrew formula, would you be interested in maintaining it/updating it in future when we cut new releases? Having to do that and MacPorts when my only Mac access is via SSH to an OLP one is a bit of a pain, and takes time away from actual development.

I actually considered whether it would be possible to fix the formula to use python2, but it would require a py2 build of protobuf3.6, which given python 2 is going unsupported (https://pythonclock.org/) etc seemed like it might be hard to get approved (though just copying the python code over works, of course)

Likewise our CI is struggling with the Protobuf changes, you can see some of my attempts to fix it in #1584 .

I have a local build with #1538 applied that resolved the problems, though as I'm deploying on a RPi I forced everything back to py2 after awhile anyway. Regardless +1 for getting the py3 compatibility shipped. Do agree with the comments on #1538 about the need for more tests especially if intention is to ship both py2 and py3 for awhile. I might be able to devote a bit of time to some of that in the next couple of months, if needed.

Yeah that would be appreciated, I don't know if @nils-van-zuijlen has been tied up with life too, but if they don't have time or don't want to add the tests then getting someone else to do so would be good.

Keeper-of-the-Keys commented 4 years ago

I don't mind having a crack at porting stuff to python3 however I have never truly used the python scripts or bindings in a meaningful way so I would need to have some defined test setup, what to run etc.

brucelowekamp commented 4 years ago

Braindumping on status and what next steps might be.

I'm blocked on #1605 (the cmp fix @nils-van-zuijlen started) b/c I don't know how to create an environment that will run debuild with the old protobuf dependency, which @peternewman asked for. Would appreciate pointers for how to get past that, even if the goal is just to have debuild run a sanity check in each supported version.

1605 I believe will complete python3 for the API SDK. BUT, there are quite a few changes needed in the remainder of the code, both the build scripts and the rdm test (install and flake8 both won't complete at present in python3). I have most of those done and they're relatively straightforward, but I don't use the rdm test suite so am unable to test them. There's one bit for unicode that needs to change quite a bit that I'll need help to understand, but otherwise I think it's just a matter of testing that they work. AFAIK there's no automated tests of the rdm test suite, though, and it's a lot of changes to be confident in without testing. (which I think is also @Keeper-of-the-Keys concern)

The other question is how to test the python code moving forward. I understand the desire to maintain support for python 2.6, but I also realized that there are quite a few python3 compatibility features that were put into 2.7, and I'm not an expert on python to know all the 2.6 vs 2.7 vs 3.7 differences. I suspect I've broken 2.7 somewhere in the changes I made for 3 in the rest of the codebase. Regardless, IMHO distcheck, or at minimum the CI, ought to run the tests against the intentionally supported python versions. It's too easy to break something across python versions. (the autoconf community refused to support multiple python versions in the same configure, and recommended using a vpath build to configure it multiple ways, which I think would be rather easy to do in distcheck and not terribly time consuming if clean isn't run in between the python versions)

I think the cmp fix ought to be merged into 0.10 so the API SDK works with python 3. I can push the rest of the changes I have to get 2.7 & 3 working, probably with distcheck. But I think automating a test of 2.6 falls into the same category of I'm not sure how to find packages for a CI system to get 2.6 (assume I could hand build it myself), and as there are no automated tests of the rdm test suite, I wouldn't want to swear 2.6 isn't broken in them. Becomes a lot easier if 2.6 is dropped, but if others know how to CI with 2.6 and test the rdm suite, I think it's possible. IMHO 2.6 should be dropped, but it's really not my call.

Keeper-of-the-Keys commented 4 years ago

// Climbs onto soap box Hi all,

The final release of python 2.6 was more then 10 years ago and it has been deprecated since, we are volunteers maintaining a package not a company that committed to supporting some archaic software distros for some SLA period, as such I have 0 qualms about dropping support for both python 2.6 and python 2.7 (which has been announced to be deprecated for years).

Ubuntu 16.04 which was mentioned as an example may still default 'python' as python 2.7 on desktop but on server images python 3.5 is 'python' (and it is also available explicitly as python3 on either): https://wiki.ubuntu.com/XenialXerus/ReleaseNotes#Python_3

If that means that the next release is called 0.11 and that there will be no further 0.10.x releases because no-one is going to support the technological baggage that is dragging the 0.10.x branch down so be it.

@peternewman is I am sure very busy next to this volunteer project and it seems to me that we are committing to a ridiculous amount of legacy drag for no good reason especially since as said all the contributors here are volunteers.

I currently have 3 open PRs on the road to RDM support in the FTDI plugin, I could open multiple additional to finish this job but I'm not going to bother if every single PR just gets bogged down in this attitude of "we need to support every single obscure and old OS out there" which absolutely does not match the time commitments anyone here can make.

In short there is no valid excuse to make any effort or ask someone else to waste their time to support python 2.6, it's borderline offensive to do so for a volunteer project, users still depending on python 2.6 can continue to use our old releases without support from us.

As for python 2.7 why encourage it? Yes it will continue to be available in a bunch of distros whose SLA demands it but all those distros ship with varying versions of python 3 and there is no reason not to use that instead of python 2.7.

Developer time is just like any other resource it can be spent once we should use it wisely and on things that will move the project forward currently we are trying to do crazy acrobatics for things that don't deserve it.

I'm looking forward to seeing this project move into the future again, the various charts tell a very sad story at the moment of practically stalled development which I have no doubt is mainly due to time constraints but also due to unreasonable demands from contributors.

// Climbs of soapbox - End rant

All contributions are greatly appreciated and the hard work by @peternewman and @nomis52 to maintain this project is too :)

nomis52 commented 4 years ago

Python 2.7 has reached EOL. I'd suggest dropping support for 2.6 and 2.7 in the next release.

brucelowekamp commented 4 years ago

I’m not a core user of OLA (found it very helpful for doing a christmas light show and being able to teach my kids a bit of programming that made lights flash, yay!). How many people need it in 2.7? At this point the general community seems to be 60/40 3.x vs 2.7 (https://pypistats.org/packages/protobuf). I think it remains to be seen if 2.7 being declared unsupported (it’s “unsupported” except for that one last release they decided to do) will move people off in the next year.

If the people using OLA python are doing quick scripts or otherwise don’t mind refactoring the code, it’s easier to just support 3. But if it would disturb users to drop 2.7 b/c they have a lot of code or integrate it with other packages, 2.7 isn’t that big of a deal IMHO.

Keeper-of-the-Keys commented 4 years ago

Python 2.7 is unsupported and EOL as of the beginning of the month, distros like RHEL7 will continue to do some security maintenance because that is their SLA and even they added a caveat in the RHEL8 release iirc that python2 would be removed during the product cycle (RHEL7 will be EOL in 2021, RHEL8 has until 2028).

The point is all these distros ship with python 3.x in the base install or easily available through their package manager so there is no reason not to switch OLA to python 3 and for the sake of compatibility we can always use the explicit 'python3' invocation, even if for some reason beyond reason we would stick with python2 we still would need to start explicitly invoking python2 since there is now a split in the distro world to which version of python the plain python command refers.

Also new distros (Ubuntu 20.04 for instance) will not ship with python2, so we are soon going to have users whose install will outright fail if we don't get this in order.

peternewman commented 4 years ago

I completely get where you're coming from @Keeper-of-the-Keys .

Personally I'm a bit frustrated we seem to be constantly playing catchup with the latest versions of stuff rather than actually writing code. @brucelowekamp sums up my feelings quite well, so I'm going to quote him:

https://github.com/OpenLightingProject/ola/pull/1605#pullrequestreview-340215810

I think leaving 2.6 supported until the next major release makes sense. Having put the boilerplate code in, it doesn't really cost anything to leave it. If someone needs to refactor again they could remove the extra comparisons and do some things like shift the protocol classes to use the attrs package rather than tons of boilerplate, but it's fine now.

I don't believe 2.7 is going anywhere :)

I know I'm in disagreement with our benevolent dictator @nomis52 :stuck_out_tongue:

Python 2.7 has reached EOL. I'd suggest dropping support for 2.6 and 2.7 in the next release.

But personally my thinking is release 0.10.8 probably as soon as #1605 is in, possibly after one or two other minor bugs. Then I suspect release 0.11.0 almost immediately as it's had a queue of shiny new features appearing since 2016 (gulp), but no-one seems to offer to test it thoroughly when I ask, branch that off for 0.11.1 if there are any bugfixes, then master becomes what will be 0.12.0 and people can start culling or breaking things if they like, such as C++11 and Python 2, rather than throw away a lot of known good compatibility and features for no good reason. Then people have a choice of a bug fix on what they're using (0.10.8), latest stable (0.11.0) or bleeding edge (master).

I don't disagree we need to switch to Python 3, but unfortunately because of the number of breaking changes they've put in it (compared to saying using a C++11 capable compiler to compile some C++98 code), we'll need to be able to support it completely before we drop Python 2, i.e. ensure all the RDM test suite and the other code works with it.

In terms of testing the RDM test suite, obviously as a test suite that's a bit of a challenge, we don't have a set of bad test case responders, but the dummy ones should all pass all the tests (I usually deliberately break them when writing a new test), so the best we can hope for really is to run all the tests against all the RDM responders (remember to enable the ack timer one in the config file as it's disabled by default), then use ola_rdm_discover to iterate through each responder in the universe the dummy plugin is patched to running the RDM test suite against that responder. I think I made the exit code from the tests correct. We should also run the model collector once against the universe. Obviously this all needs to be done with OLAd running and the dummy plugin port patched to a universe. I think I started writing a script to do some of that once but I'd have to dig to see if I could find it again...

Also obviously everyone's contributions are brilliant, sorry I've been so busy (mostly with $work) and failing to review stuff effectively. I'm really looking forward to getting your FTDI RDM stuff in @Keeper-of-the-Keys I think it could be a real game changer.

brucelowekamp commented 4 years ago

Maybe

For 0.10.8 merge #1605. I believe that makes the sdk libraries python 3 compatible (though they would need to be installed there manually) without breaking either 2.6 or 2.7. (I wouldn't be surprised if the new tests don't break in 2.6, but they could just be commented out if someone really wants to run check under 2.6)

For 0.11 support 2.7 and 3.>7. Add a travis builder (change one of the native builders) to have only python3 and make sure one of the others has 2.7. Don't change the build system to build (or check) both. Maybe do something in debuild etc to package both, but nothing other than that.

Beyond 0.11 see what happens with the community, whether cost of supporting 2.7 is high and there is an audience.

Bruce

Keeper-of-the-Keys commented 4 years ago

Releasing a new "major" version (0.11) with Python 2 support/dependency is madness, Python 2 is dead, Python 3 is available on all systems that may or may not still carry Python 2.

Definitely if we consider just how long the 0.10 line is around already, if 0.11 is just a temporary/in between release that will have maybe 1 point release to get Python 3 in order at a nicer pace fine but if it's intended to be around for several years then absolutely not.

peternewman commented 4 years ago

Releasing a new "major" version (0.11) with Python 2 support/dependency is madness, Python 2 is dead, Python 3 is available on all systems that may or may not still carry Python 2.

I'm just trying to be pragmatic @Keeper-of-the-Keys , we should have released 0.11 a long time ago, but we keep getting stuck fixing bugs in 0.10 branch. Waiting until we've completely ported 0.11 to Python 3 seems like a waste of good code to me (with lots of exciting new functionality!).

Definitely if we consider just how long the 0.10 line is around already, if 0.11 is just a temporary/in between release that will have maybe 1 point release to get Python 3 in order at a nicer pace fine but if it's intended to be around for several years then absolutely not.

I suspect ideally our major releases should be say once a year or possibly every sixth months, depending on what's going into them (although looking at the release history, yearly is probably most likely as we're only normally getting any release out every 6 months). The fact we've not even had a point release for 18 months is not great really! :cry:

Keeper-of-the-Keys commented 4 years ago

Due to some issues I was having with Ubuntu 19.10 I just upgraded to 20.04 and as expected Python 2.x is no longer available, it gets removed during the upgrade process and does not exist in the repositories for Ubuntu 20.04.

On checking OLA is currently also not available in the 20.04 repositories.

yoe commented 4 years ago

Ola is currently not available in ubuntu 20.04 because of the python2 removal, and it's probably too late to get it back by now. The same has already happened for bullseye (the next Debian release), although there we still have time to get a new version in.

There's https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=937186 which tracks this for ola in Debian.

brucelowekamp commented 4 years ago

I found out recently there are changes in master in the python code that needs to be changed that aren’t in 0.10. I have maybe most of the work for the rest of python3 in a branch, but would rather not try to put it into a PR until 1605 is merged into master. I’m happy to push what I have and then the rest of the work would likely benefit from folks collaborating to iron it out.

peternewman commented 4 years ago

The first part is in via #1605 . Some outstanding todos below:

peternewman commented 1 year ago

@brucelowekamp and anyone else, if you've got time I'd appreciate people taking a look at #1761 which I think fixes pretty much all of the functional issues with Python 3 support (finally)!

@nomis52 I'd welcome any thoughts on the fact I'm now supporting Unicode (already, before the new E1.20 is out), and also breaking our past behaviour for characters greater than 0x7f...

I've also just spotted the list in https://github.com/OpenLightingProject/ola/issues/1506#issuecomment-620310223 which is probably still outstanding...