Skip to content

Commit

Permalink
Don't repeat the cmp function
Browse files Browse the repository at this point in the history
  • Loading branch information
eric-wieser authored and dirk-thomas committed Jun 24, 2016
1 parent 18620c1 commit 085e265
Showing 1 changed file with 4 additions and 18 deletions.
22 changes: 4 additions & 18 deletions src/genpy/rostime.py
Expand Up @@ -181,12 +181,8 @@ def __ne__(self, other):
def __cmp__(self, other):
if not isinstance(other, TVal):
raise TypeError("Cannot compare to non-TVal")
nanos = self.to_nsec() - other.to_nsec()
if nanos > 0:
return 1
if nanos == 0:
return 0
return -1
return cmp(self.to_nsec(), other.to_nsec())

def __eq__(self, other):
if not isinstance(other, TVal):
return False
Expand Down Expand Up @@ -266,12 +262,7 @@ def __cmp__(self, other):
"""
if not isinstance(other, Time):
raise TypeError("cannot compare to non-Time")
nanos = self.to_nsec() - other.to_nsec()
if nanos > 0:
return 1
if nanos == 0:
return 0
return -1
return cmp(self.to_nsec(), other.to_nsec())

def __eq__(self, other):
"""
Expand Down Expand Up @@ -414,12 +405,7 @@ def __truediv__(self, val):
def __cmp__(self, other):
if not isinstance(other, Duration):
raise TypeError("Cannot compare to non-Duration")
nanos = self.to_nsec() - other.to_nsec()
if nanos > 0:
return 1
if nanos == 0:
return 0
return -1
return cmp(self.to_nsec(), other.to_nsec())

def __eq__(self, other):
if not isinstance(other, Duration):
Expand Down

9 comments on commit 085e265

@islers
Copy link

@islers islers commented on 085e265 Oct 18, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi guys! We know that in ROS Kinetic you officially only support Python 2.x. But much of your code is actually compatible to be run under Python 3.x - We appreciate all your hard work and I know this is actually our fault since we relied on something that is not to be relied on by the specs but it would be nice if you didn't break any such compatibility with "updates" anymore in the future. The update with this change broke our rospy dependent code which actually ran fine before under Python 3.5...

@dirk-thomas
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From you comment I couldn't extract yet what the problem is except that something broke for you with Python 3.5. Can you please describe what exactly changed behavior and broke compatibility.

@islers
Copy link

@islers islers commented on 085e265 Oct 19, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is rather simple: The "cmp" function doesn't exist anymore in Python 3 (see https://docs.python.org/3.0/whatsnew/3.0.html)

@dirk-thomas
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for clarifying. Since we don't run any tests with Python 3 that went in without being notices. I have created #71 to address this (as well as a couple of other issues). Can you please confirm that the branch makes it work for you with Python 3 again.

@islers
Copy link

@islers islers commented on 085e265 Oct 19, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm rather impressed by the quick response, thank you! I didn't test the referenced branch but chose the quicker way of testing the new rostime.py by copying it into my current installation: I can confirm that it resolves the Python 3 issue.

@dirk-thomas
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for testing the change. I guess it is soon time for a new genpy release then... 😞

@eric-wieser
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@islers: How are you ending up in a state where cmp even gets executed?

@islers
Copy link

@islers islers commented on 085e265 Oct 24, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well I can tell you where and why, but I didn't debug the complete process stack to tell you how.

@eric-wieser
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't worry about it, this was resolved in another comment thread

Please sign in to comment.