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

[Feature] Flake8' tool to perform additional formatting and semantic checking of code. #20429

Closed
mozga-intel wants to merge 10 commits into from
Closed

[Feature] Flake8' tool to perform additional formatting and semantic checking of code. #20429

mozga-intel wants to merge 10 commits into from

Conversation

@mozga-intel
Copy link
Contributor

@mozga-intel mozga-intel commented Jul 9, 2021 •
edited
Loading

Description

There is used flake8' tool to perform additional formatting and semantic checking of code. This pull request contains a pre-commit git hook, a requirement file, .flake8 file with a flake8 configuration. (Related to: #20406, #20356)

  1. Since Flake8 needs to be installed with an appropriate configuration, the file with the requirements has to be run:
pip install -r requirements-flake8.txt
  1. After that, we need to set up a symbolic link to a given file: To create a symbolic link to a given file, open your terminal and type:
ln -s ../../tools/git-pre-commit .git/hooks/pre-commit
  1. If you have not set up the pre-commit hooks, you can run it locally in your local branch.

Future consideration

  1. Flake8 tool should be run also on a CI.
  2. *.py files need to be adjusted firstly. Flake8 check the entire file.
  3. Add a pre-commit configuration: [pre-commit manager]
    3.1: Create a file named .pre-commit -config.yaml.
    3.2: Set up the full set of options for a given file.

Checklist

Essentials

  • PR's title starts with a category (e.g. [BUGFIX], [MODEL], [TUTORIAL], [FEATURE], [DOC], etc)
  • Changes are complete (i.e. I finished coding on this PR)
  • All changes have test coverage
  • Code is well-documented

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

@mozga-intel mozga-intel requested a review from szha as a code owner July 9, 2021 11:59
@mxnet-bot
Copy link

mxnet-bot commented Jul 9, 2021

Hey @mozga-intel , Thanks for submitting the PR
All tests are already queued to run once. If tests fail, you can trigger one or more tests again with the following commands:

  • To trigger all jobs: @mxnet-bot run ci [all]
  • To trigger specific jobs: @mxnet-bot run ci [job1, job2]

CI supported jobs: [windows-gpu, unix-cpu, unix-gpu, website, clang, windows-cpu, centos-gpu, edge, centos-cpu, sanity, miscellaneous]

Note:
Only following 3 categories can trigger CI :PR Author, MXNet Committer, Jenkins Admin.
All CI tests must pass before the PR can be merged.

@mseth10 mseth10 added the pr-work-in-progress PR is still work in progress label Jul 9, 2021
This was referenced Jul 9, 2021
Merged
Merged
@mozga-intel
Copy link
Contributor Author

mozga-intel commented Jul 13, 2021

@mxnet-bot run ci [unix-gpu]

@mxnet-bot
Copy link

mxnet-bot commented Jul 13, 2021

Jenkins CI successfully triggered : [unix-gpu]

@mozga-intel mozga-intel changed the title [WIP] Flake8' tool to perform additional formatting and semantic checking of code. Flake8' tool to perform additional formatting and semantic checking of code. Jul 30, 2021
@mseth10 mseth10 added pr-awaiting-testing PR is reviewed and waiting CI build and test pr-work-in-progress PR is still work in progress and removed pr-work-in-progress PR is still work in progress pr-awaiting-testing PR is reviewed and waiting CI build and test labels Jul 30, 2021
@mxnet-bot
Copy link

mxnet-bot commented Aug 3, 2021

Jenkins CI successfully triggered : [sanity]

@mozga-intel
Copy link
Contributor Author

mozga-intel commented Aug 3, 2021

@mxnet-bot run ci [all]

@mxnet-bot
Copy link

mxnet-bot commented Aug 3, 2021

Jenkins CI successfully triggered : [miscellaneous, centos-gpu, windows-cpu, unix-cpu, sanity, edge, windows-gpu, website, centos-cpu, unix-gpu, clang]

@mseth10 mseth10 added pr-awaiting-testing PR is reviewed and waiting CI build and test pr-work-in-progress PR is still work in progress and removed pr-work-in-progress PR is still work in progress pr-awaiting-testing PR is reviewed and waiting CI build and test labels Aug 3, 2021
@mozga-intel
Copy link
Contributor Author

mozga-intel commented Aug 5, 2021 •
edited
Loading

Following the pull-request description, running flake8 on the current master produces a bunch of errors (pasted below). At some point of time, some checking has not been enabled yet. Please have a look at the log below:

[2021-08-04T10:47:16.901Z] ./tests/python/gpu/test_numpy_fallback.py:54:31: E231 missing whitespace after ','

[2021-08-04T10:47:16.901Z] ./tests/python/gpu/test_numpy_fallback.py:54:34: E225 missing whitespace around operator

[2021-08-04T10:47:16.901Z] ./tests/python/gpu/test_numpy_fallback.py:64:27: E231 missing whitespace after ','

[2021-08-04T10:47:16.901Z] ./tests/python/gpu/test_numpy_fallback.py:64:31: E231 missing whitespace after ','

[2021-08-04T10:47:16.901Z] ./tests/python/gpu/test_numpy_fallback.py:64:34: E225 missing whitespace around operator

[2021-08-04T10:47:16.901Z] ./tests/python/gpu/test_numpy_fallback.py:70:12: F632 use ==/!= to compare constant literals (str, bytes, int, float, tuple)

[2021-08-04T10:47:16.901Z] ./tests/python/gpu/test_numpy_fallback.py:73:40: C416 Unnecessary list comprehension - rewrite using list().

[2021-08-04T10:47:16.901Z] ./tests/python/gpu/test_numpy_fallback.py:78:24: E231 missing whitespace after ','

[2021-08-04T10:47:16.901Z] ./tests/python/gpu/test_numpy_fallback.py:78:26: E231 missing whitespace after ','

[2021-08-04T10:47:16.901Z] ./tests/python/gpu/test_numpy_fallback.py:78:29: E231 missing whitespace after ','

[2021-08-04T10:47:16.901Z] ./tests/python/gpu/test_numpy_fallback.py:78:32: E231 missing whitespace after ','

To pass sanity-check, adjusting those ones to the flake8' report is needed.

@mozga-intel mozga-intel force-pushed the mozga-intel/flake8_python_checker branch from 9208520 to e52a662 Compare October 20, 2021 11:23
@mseth10 mseth10 added pr-awaiting-testing PR is reviewed and waiting CI build and test pr-work-in-progress PR is still work in progress and removed pr-work-in-progress PR is still work in progress pr-awaiting-testing PR is reviewed and waiting CI build and test labels Oct 20, 2021
@mozga-intel mozga-intel force-pushed the mozga-intel/flake8_python_checker branch from 6f583bf to 08af07d Compare October 20, 2021 16:02
@mseth10 mseth10 added pr-awaiting-testing PR is reviewed and waiting CI build and test pr-work-in-progress PR is still work in progress and removed pr-work-in-progress PR is still work in progress pr-awaiting-testing PR is reviewed and waiting CI build and test labels Oct 20, 2021
@mozga-intel
Copy link
Contributor Author

mozga-intel commented Oct 20, 2021

Close this PR as it is flake8 is replaced by #20684

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@szha szha Awaiting requested review from szha szha is a code owner

@aaronmarkham aaronmarkham Awaiting requested review from aaronmarkham aaronmarkham is a code owner

@marcoabreu marcoabreu Awaiting requested review from marcoabreu marcoabreu is a code owner

Assignees

No one assigned

Labels

pr-work-in-progress PR is still work in progress

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mozga-intel @mxnet-bot @mseth10