-
Notifications
You must be signed in to change notification settings - Fork 6.8k
[MXNET-696] Define cmp() in Python 3 again #12295
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Thanks for fixing the pylint issue here.
@@ -29,6 +29,13 @@ | |||
import re | |||
from mxnet.test_utils import assert_almost_equal | |||
|
|||
try: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cmp = lambda(x, y): (x > y) - (x < y)
could also work with a bit less code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- It is a Python 3 syntax error
- With the syntax fixed it generates a PEP8 violation in flake8 https://stackoverflow.com/questions/25010167/e731-do-not-assign-a-lambda-expression-use-a-def
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I see. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contributions!
This PR is ready to merge after CI pass.
I have restarted the PR build as it earlier failed with file download transient issue.
Description
A second attempt at #12191 with no Unicode characters this time...
Fixes #8270 @vandanavk
cmp() was removed in Python 3 so this PR recreates it if required leveraging the formula at http://python-future.org/compatible_idioms.html#cmp
Description
Define cmp() in Python 3
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.
Changes
Comments
@marcoabreu #12237 (comment)