Update ClientWrapper.py #1514
Update ClientWrapper.py #1514
Conversation
Added the comparison to Event class so examples are compatible with python3 :
def __lt__(self,other):
return self._run_at< other._run_at
There was a problem hiding this comment.
Hi @jimruxton ,
Thanks for this and congratulations on your first pull request!
After looking at what we've done elsewhere, it would actually be good to target this at our 0.10 branch, so would you mind doing the same edit against this link, then do a new Pull Request but select our 0.10 branch on the left and it should work as intended:
https://github.com/OpenLightingProject/ola/blob/0.10/python/ola/ClientWrapper.py
We also run Travis continuous integration which has found a few minor issues with your changes, see them here (I've also commented against the relevant lines and offered suggested fixes):
https://travis-ci.org/OpenLightingProject/ola/jobs/457080510#L1166
| @@ -46,6 +46,9 @@ def __init__(self, time_ms, callback): | |||
|
|
|||
| def __cmp__(self, other): | |||
| return cmp(self._run_at, other._run_at) | |||
|
|
|||
There was a problem hiding this comment.
https://travis-ci.org/OpenLightingProject/ola/jobs/457080510#L1166
python/ola/ClientWrapper.py:49:1: W293 blank line contains whitespace
| @@ -46,6 +46,9 @@ def __init__(self, time_ms, callback): | |||
|
|
|||
| def __cmp__(self, other): | |||
| return cmp(self._run_at, other._run_at) | |||
|
|
|||
| def __lt__(self,other): | |||
There was a problem hiding this comment.
https://travis-ci.org/OpenLightingProject/ola/jobs/457080510#L1167
python/ola/ClientWrapper.py:50:18: E231 missing whitespace after ','
| def __lt__(self,other): | |
| def __lt__(self, other): |
| @@ -46,6 +46,9 @@ def __init__(self, time_ms, callback): | |||
|
|
|||
| def __cmp__(self, other): | |||
| return cmp(self._run_at, other._run_at) | |||
|
|
|||
| def __lt__(self,other): | |||
| return self._run_at< other._run_at | |||
There was a problem hiding this comment.
https://travis-ci.org/OpenLightingProject/ola/jobs/457080510#L1168
python/ola/ClientWrapper.py:51:24: E225 missing whitespace around operator
| return self._run_at< other._run_at | |
| return self._run_at < other._run_at |
|
Hi @jimruxton , are you interested in trying to fix the comments either for our master branch or ideally for our 0.10 branch? I think they should be fairly easy to do (I've not tried it, but I think in theory you can take the suggested fixes from my comments and easily integrate them, see https://help.github.com/articles/incorporating-feedback-in-your-pull-request/#applying-a-suggested-change ). |
|
I'm closing this as it's been superceded by @brucelowekamp and others work in #1605 which is in 0.10 and will be in master shortly. Thanks for your efforts on it anyway @jimruxton . |
Added the comparison to Event class so examples are compatible with python3 :
def lt(self,other):
return self._run_at< other._run_at