Some Python 3 fixes in ./example#11671
Some Python 3 fixes in ./example#11671zhreshold merged 2 commits intoapache:masterfrom cclauss:fix-examples-for-Python3
Conversation
|
Pinging @antinucleon @precedenceguo @WellyZhang @zhreshold, respective example contributors, in case there are concerns. |
| if PYTHON_VERSION == 2: | ||
| from urllib import urlretrieve | ||
| elif PYTHON_VERSION == 3: | ||
| from six.moves import string_types |
There was a problem hiding this comment.
I think six is existing dependency of mxnet. For sring_types you can use mxnet.base.string_types
There was a problem hiding this comment.
This file does not import mxnet.
There was a problem hiding this comment.
I am trying to follow the advise in #10833 (comment)
There was a problem hiding this comment.
I agree with #10833, since mxnet is always a dependency, so I guess import it and use mxnet.base.string_types is better, otherwise users will complain it six is not installed.
|
I see 2 pycocotools files changed. I would suggest removing copied version of pycocotools from both ssd and rcnn examples. The pip version works just fine in gluon-cv. I did update the rcnn example at #11373. Seems like a huge diff but the core logic did not change at all although duplicate codes are gone. |
|
@zhreshold @ijkguo Did you want to see further changes made? |
|
No change necessary on rcnn example. I tested with Python 3. |
|
Agreed. I had removed the rcnn example from this PR as you suggested. |
| import random | ||
| import numpy as np | ||
| import argparse | ||
| from six.moves import xrange |
There was a problem hiding this comment.
just replace xrange with range for python3 compatibility, since there's no performance issue here.
| import pyprind | ||
| import mxnet as mx | ||
| import numpy as np | ||
| from mxnet.base import xrange |
| if PYTHON_VERSION == 2: | ||
| from urllib import urlretrieve | ||
| elif PYTHON_VERSION == 3: | ||
| from six.moves import string_types |
There was a problem hiding this comment.
I agree with #10833, since mxnet is always a dependency, so I guess import it and use mxnet.base.string_types is better, otherwise users will complain it six is not installed.
|
@cclauss Thanks for catching these compatibility issues, we can merge once these comments are addressed! |
|
Any pointers on why the tests are failing? |
|
Yes... It succeeded after rebasing. |
* Some Python 3 fixes in ./example * mxnet.base.string_types and xrange() --> range()
* Some Python 3 fixes in ./example * mxnet.base.string_types and xrange() --> range()
* Some Python 3 fixes in ./example * mxnet.base.string_types and xrange() --> range()
* Some Python 3 fixes in ./example * mxnet.base.string_types and xrange() --> range()
4 participants
Description
Fix undefined names found be flake8 F821 that are mostly (but not exclusively) related to Python 3. Each undefined name has the potential to raise NameError at runtime. Used the six module as discussed at #10833 (comment) Also see #11669
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.
Changes
Comments