Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

[MXNET-1411] solve pylint error issue#14851 #15113

Merged
merged 21 commits into from Jul 21, 2019

Conversation

cchung100m
Copy link
Contributor

Description

Pylint error 'Bad option value 'no-else-raise' (bad-option-value)' was reported by issue 14851, therefore, I check the code and remove the statement '# pylint: disable=redefined-variable-type' for solving the issue.

My environment:
(venv) cch:incubator-mxnet cch$ pylint --version
pylint 2.1.1
astroid 2.1.0
Python 3.5.1 (default, Jan 22 2016, 17:08:33)
[GCC 4.2.1 Compatible Apple LLVM 6.0 (clang-600.0.57)]

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

  • The PR title starts with [MXNET-1411], where MXNET-1411 refers to the relevant JIRA issue created (except PRs with tiny changes)
  • Changes are complete (i.e. I finished coding on this PR)
  • All changes have test coverage:
  • Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
  • Nightly tests are added for complicated/long-running ones (e.g. changing distributed kvstore)
  • Build tests will be added for build configuration changes (e.g. adding a new build option with NCCL)
  • Code is well-documented:
  • For user-facing API changes, API doc string has been updated.
  • For new C++ functions in header files, their functionalities and arguments are documented.
  • For new examples, README.md is added to explain the what the example does, the source of the dataset, expected performance on test set and reference to the original paper if applicable
  • Check the API doc at http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
  • To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

  • Feature1, tests, (and when applicable, API doc)
  • Feature2, tests, (and when applicable, API doc)

Comments

  • If this change is a backward incompatible change, why must this change be made.
  • Interesting edge cases to note here

@cchung100m
Copy link
Contributor Author

Hi @stu1130,

Follow the discussion from issue page, I remove the statement '# pylint: disable=redefined-variable-type' for solving the issue. I would appreciate if you can give me some suggestions, thanks.

@abhinavs95
Copy link
Contributor

@mxnet-label-bot add [pr-awaiting-review]

@marcoabreu marcoabreu added the pr-awaiting-review PR is waiting for code review label May 31, 2019
@stu1130
Copy link
Contributor

stu1130 commented May 31, 2019

Thanks for your contribution @cchung100m.
Could you use make pylint to check if it pass the pylint?

@cchung100m
Copy link
Contributor Author

Hi @stu1130

Here is the pylint check, please help to review the result, thanks.

@cchung100m
Copy link
Contributor Author

cchung100m commented Jun 8, 2019

Hi @stu1130, @marcoabreu

I cannot pass the Jenkins' sanity check job, I would appreciate if you can give me suggestions to solve it, thanks.

Makefile:600: recipe for target 'pylint' failed

make: *** [pylint] Error 8

build.py: 2019-06-08 03:54:08,362Z INFO Waiting for status of container 61d8fff15086 for 600 s.

build.py: 2019-06-08 03:54:08,572Z INFO Container exit status: {'StatusCode': 2, 'Error': None}

build.py: 2019-06-08 03:54:08,572Z ERROR Container exited with an error 😞

build.py: 2019-06-08 03:54:08,572Z INFO Executed command for reproduction:



ci/build.py --docker-registry mxnetci --platform ubuntu_cpu --docker-build-retries 3 --shm-size 500m /work/runtime_functions.sh sanity_check



build.py: 2019-06-08 03:54:08,572Z INFO Stopping container: 61d8fff15086

build.py: 2019-06-08 03:54:08,574Z INFO Removing container: 61d8fff15086

build.py: 2019-06-08 03:54:08,593Z INFO Other running containers: ['33d483cb37cd', 'd5b3b1c3352c']

build.py: 2019-06-08 03:54:08,593Z CRITICAL Execution of ['/work/runtime_functions.sh', 'sanity_check'] failed with status: 2

Terminated

script returned exit code 2

@marcoabreu
Copy link
Contributor

make pylint

python3 -m pylint --rcfile=/work/mxnet/ci/other/pylintrc --ignore-patterns=".*\.so$,.*\.dll$,.*\.dylib$" python/mxnet tools/caffe_converter/*.py

************* Module mxnet.contrib.text.vocab

python/mxnet/contrib/text/vocab.py:213:12: R1720: Unnecessary "else" after "raise" (no-else-raise)

************* Module mxnet.model

python/mxnet/model.py:645:16: R1720: Unnecessary "else" after "raise" (no-else-raise)

************* Module mxnet.contrib.onnx.onnx2mx._translation_utils

python/mxnet/contrib/onnx/onnx2mx/_translation_utils.py:181:4: R1720: Unnecessary "else" after "raise" (no-else-raise)

************* Module mxnet.contrib.onnx.mx2onnx._export_helper

python/mxnet/contrib/onnx/mx2onnx/_export_helper.py:43:4: R1720: Unnecessary "else" after "raise" (no-else-raise)

************* Module mxnet.test_utils

python/mxnet/test_utils.py:214:4: R1720: Unnecessary "else" after "raise" (no-else-raise)

python/mxnet/test_utils.py:1409:16: R1720: Unnecessary "else" after "raise" (no-else-raise)

python/mxnet/test_utils.py:1436:20: R1720: Unnecessary "else" after "raise" (no-else-raise)

python/mxnet/test_utils.py:1516:12: R1720: Unnecessary "else" after "raise" (no-else-raise)

python/mxnet/test_utils.py:1597:11: R1719: The if expression can be replaced with 'not test' (simplifiable-if-expression)

************* Module mxnet.image.image

python/mxnet/image/image.py:1377:12: R1720: Unnecessary "elif" after "raise" (no-else-raise)

************* Module mxnet.gluon.trainer

python/mxnet/gluon/trainer.py:252:8: R1720: Unnecessary "else" after "raise" (no-else-raise)

python/mxnet/gluon/trainer.py:273:8: R1720: Unnecessary "else" after "raise" (no-else-raise)

************* Module mxnet.image.detection

python/mxnet/image/detection.py:812:12: R1720: Unnecessary "elif" after "raise" (no-else-raise)

************* Module mxnet.gluon.utils

python/mxnet/gluon/utils.py:344:16: R1720: Unnecessary "else" after "raise" (no-else-raise)

************* Module mxnet.ndarray.sparse

python/mxnet/ndarray/sparse.py:642:12: R1720: Unnecessary "else" after "raise" (no-else-raise)

python/mxnet/ndarray/sparse.py:1105:8: R1720: Unnecessary "elif" after "raise" (no-else-raise)



-----------------------------------

Your code has been rated at 9.99/10

@cchung100m
Copy link
Contributor Author

Hi @marcoabreu

Thanks for the suggestions.

I improve the PR by solving pylint error R1720, but still fail with CI job 'sanity',

+ make pylint

python3 -m pylint --rcfile=/work/mxnet/ci/other/pylintrc --ignore-patterns=".*\.so$,.*\.dll$,.*\.dylib$" python/mxnet tools/caffe_converter/*.py

************* Module mxnet.test_utils

python/mxnet/test_utils.py:1597:11: R1719: The if expression can be replaced with 'not test' (simplifiable-if-expression)

------------------------------------

Your code has been rated at 10.00/10

However, I check the test_utils.py with the following command but didn't find the error.

(venv) cch:incubator-mxnet cch$ python3 -m pylint --rcfile=ci/other/pylintrc --ignore-patterns=".*\.so$,.*\.dll$,.*\.dylib$" python/mxnet/test_utils.py 
--------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)

I would appreciate if you can give me more clues, thanks :)

@cchung100m
Copy link
Contributor Author

Hi @stu1130, @marcoabreu

I cannot find the error "The if expression can be replaced with 'not test'" in Module mxnet.test_utils. I would appreciate if you can give me suggestions to solve it, thanks.

@roywei
Copy link
Member

roywei commented Jul 8, 2019

@stu1130 could you help? thanks!

@karan6181
Copy link
Contributor

@stu1130, @marcoabreu Could you please help the author on resolving the issue? Thanks!

@marcoabreu
Copy link
Contributor

Hi, it could be possible that you are running an old version of pylint. Could you check that? Sorry for the delay

@cchung100m
Copy link
Contributor Author

@marcoabreu
Thanks for suggestions, I upgrade the version of pylint from 2.1.1 to 2.3.1 and fix the issue.
Please help to review/merge, thanks :)

@marcoabreu marcoabreu merged commit d599bc3 into apache:master Jul 21, 2019
anirudhacharya pushed a commit to anirudhacharya/mxnet that referenced this pull request Aug 20, 2019
* fix pylint error: no-else-raise in _export_helper.py

* fix pylint error: no-else-raise in _translation_utils.py

* fix pylint error: Bad option value 'no-else-raise' (bad-option-value) in vocab.py

* fix pylint error: Bad option value 'no-else-raise' (bad-option-value) in trainer.py

* fix pylint error: Bad option value 'no-else-raise' (bad-option-value) in utils.py

* fix pylint error: Bad option value 'no-else-raise' (bad-option-value) in detection.py

* fix pylint error: Bad option value 'no-else-raise' (bad-option-value) in image.py

* fix pylint error: Bad option value 'no-else-raise' (bad-option-value) in model.py

* fix pylint error: Bad option value 'no-else-raise' (bad-option-value) in sparse.py

* fix pylint error: Bad option value 'no-else-raise' (bad-option-value) in test_utils.py

* fix pylint error: R1720: Unnecessary else after raise (no-else-raise) for vocab.py

* fix pylint error: R1720: Unnecessary else after raise (no-else-raise) for model.py

* fix pylint error: R1720: Unnecessary else after raise (no-else-raise) for _translation_utils.py

* fix pylint error: R1720: Unnecessary else after raise (no-else-raise) for _export_helper.py

* fix pylint error: R1720: Unnecessary else after raise (no-else-raise) for test_utils.py

* fix pylint error: R1720: Unnecessary else after raise (no-else-raise) for image.py

* fix pylint error: R1720: Unnecessary else after raise (no-else-raise) for trainer.py

* fix pylint error: R1720: Unnecessary else after raise (no-else-raise) for detection.py

* fix pylint error: R1720: Unnecessary else after raise (no-else-raise) for utils.py

* fix pylint error: R1720: Unnecessary else after raise (no-else-raise) for sparse.py

* fix pylint error:R1719: The if expression can be replaced with 'bool(test)' (simplifiable-if-expression)
@cchung100m cchung100m deleted the pylint_error_issue14851 branch September 4, 2019 15:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pr-awaiting-review PR is waiting for code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants