You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mxnet.apache.org by GitBox <gi...@apache.org> on 2020/02/13 02:41:26 UTC
[GitHub] [incubator-mxnet] leezu opened a new pull request #17583: Python 2
cleanup
leezu opened a new pull request #17583: Python 2 cleanup
URL: https://github.com/apache/incubator-mxnet/pull/17583
## Description ##
This PR drops some Python 2 compatibility code from the codebase. Makes the code easier to read.
### Changes ###
- [X] Drop some python 2 compatibility handlers from mxnet python package and tests
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] leezu commented on a change in pull request
#17583: Python 2 cleanup
Posted by GitBox <gi...@apache.org>.
leezu commented on a change in pull request #17583: Python 2 cleanup
URL: https://github.com/apache/incubator-mxnet/pull/17583#discussion_r378662806
##########
File path: ci/docker/runtime_functions.sh
##########
@@ -43,11 +43,10 @@ scala_prepare() {
check_cython() {
set -ex
- local python_ver=$1
- local is_cython_used=$(python${python_ver} <<EOF
+ local is_cython_used=$(python3 <<EOF
Review comment:
I think the closing brace comes in line 52.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] ChaiBapchya commented on a change in pull request
#17583: Python 2 cleanup
Posted by GitBox <gi...@apache.org>.
ChaiBapchya commented on a change in pull request #17583: Python 2 cleanup
URL: https://github.com/apache/incubator-mxnet/pull/17583#discussion_r378651541
##########
File path: ci/docker/runtime_functions.sh
##########
@@ -43,11 +43,10 @@ scala_prepare() {
check_cython() {
set -ex
- local python_ver=$1
- local is_cython_used=$(python${python_ver} <<EOF
+ local is_cython_used=$(python3 <<EOF
Review comment:
I thought it should be
```suggestion
local is_cython_used=$(python3) <<EOF
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] leezu commented on a change in pull request
#17583: Python 2 cleanup
Posted by GitBox <gi...@apache.org>.
leezu commented on a change in pull request #17583: Python 2 cleanup
URL: https://github.com/apache/incubator-mxnet/pull/17583#discussion_r378643783
##########
File path: docs/python_docs/python/tutorials/packages/gluon/text/gnmt.rst
##########
@@ -391,7 +390,7 @@ testing datasets.
def write_sentences(sentences, file_path):
- with io.open(file_path, 'w', encoding='utf-8') as of:
+ with open(file_path, 'w', encoding='utf-8') as of:
Review comment:
No. Default is platform dependent. https://docs.python.org/3/library/functions.html#open
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] ChaiBapchya commented on a change in pull request
#17583: Python 2 cleanup
Posted by GitBox <gi...@apache.org>.
ChaiBapchya commented on a change in pull request #17583: Python 2 cleanup
URL: https://github.com/apache/incubator-mxnet/pull/17583#discussion_r378644844
##########
File path: ci/docker/runtime_functions.sh
##########
@@ -43,11 +43,10 @@ scala_prepare() {
check_cython() {
set -ex
- local python_ver=$1
- local is_cython_used=$(python${python_ver} <<EOF
+ local is_cython_used=$(python3 <<EOF
Review comment:
does this work with incomplete var?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] leezu commented on a change in pull request
#17583: Python 2 cleanup
Posted by GitBox <gi...@apache.org>.
leezu commented on a change in pull request #17583: Python 2 cleanup
URL: https://github.com/apache/incubator-mxnet/pull/17583#discussion_r378646079
##########
File path: ci/docker/runtime_functions.sh
##########
@@ -43,11 +43,10 @@ scala_prepare() {
check_cython() {
set -ex
- local python_ver=$1
- local is_cython_used=$(python${python_ver} <<EOF
+ local is_cython_used=$(python3 <<EOF
Review comment:
What do you mean? `check_cython` is only called at one place and I modified that place to not pass the `python_ver`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] leezu merged pull request #17583: Python 2 cleanup
Posted by GitBox <gi...@apache.org>.
leezu merged pull request #17583: Python 2 cleanup
URL: https://github.com/apache/incubator-mxnet/pull/17583
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] larroy commented on a change in pull request
#17583: Python 2 cleanup
Posted by GitBox <gi...@apache.org>.
larroy commented on a change in pull request #17583: Python 2 cleanup
URL: https://github.com/apache/incubator-mxnet/pull/17583#discussion_r378642977
##########
File path: docs/python_docs/python/tutorials/packages/gluon/text/gnmt.rst
##########
@@ -391,7 +390,7 @@ testing datasets.
def write_sentences(sentences, file_path):
- with io.open(file_path, 'w', encoding='utf-8') as of:
+ with open(file_path, 'w', encoding='utf-8') as of:
Review comment:
Isn't utf8 by default?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services