Makefile Linter | Python & Javascript

[BluThaitanium]

Context

Enforce code-style by creating a linting target in the Makefile. Lints Python and Javascript. Addressed Issue #322

Changes

• Separate make calls for Python (.py) and Javascript (.js, .jsx, .ts, .tsx) ** Called via make lint_python and make lint_javascript ** Linting via flake8 and eslint
• ESLint formatting rules stored in /dashboard/origin-mlx/.eslintrc.yml
• Formatting changes to all .py files to meet flake8-PEP8 standards
• Formatting changes to many javascript files
• Automatic changes to package-lock.json and package.json for ReactJS support for eslint

Notes

• .eslintrc.yml's overrides parameters store rules that require large changes to the current codebase. These rules are therefore disabled.
• Linting python files requires a virtual environment stored as venv in the main /mlx/ directory

[ckadner]

Wow! Thank you Phu. This is a massive change. I might need some more time to properly review :-)

A few quick observations:

• Many of the Python code style violations are in generated files which we should probably exclude
• We should probably add some sand-boxing for Python based make targets, using a venv, like you mentioned
• Did you use autopep8 or some other tool to automatically fix some of the Python code style violations? If so, maybe we can add a script or Make target for future "auto-corrections"

[BluThaitanium]

Wow! Thank you Phu. This is a massive change. I might need some more time to properly review :-)

A few quick observations:

• Many of the Python code style violations are in generated files which we should probably exclude
• We should probably add some sand-boxing for Python based make targets, using a venv, like you mentioned
• Did you use autopep8 or some other tool to automatically fix some of the Python code style violations? If so, maybe we can add a script or Make target for future "auto-corrections"

• I'll take a look and see how to exclude those files/directories with flake8
• I'll see if we could generate a venv for the user if they don't have one
• Some of the python corrections were fixed with the Black linter. A lot of the other corrections I sifted through manually and fixed, or added # noqa.

[ckadner]

Hi @BluThaitanium -- maybe we can add another Make target to actually fix the Python code violations. We could collaborate on that (seeing that I accidentally already committed onto your branch 8-)

• autopep8 appears to work okay and understands the same codes and uses similar parameters
• autoflake takes care of unused imports and unused variables

i.e.:

.PHONY: format_python
format_python: venv ## Correct Python code style violations
	@which autoflake > /dev/null || pip install autoflake
	@which autopep8 > /dev/null || pip install autopep8
	@autoflake api tools/python -i --recursive --remove-all-unused-imports \
		--ignore-init-module-imports --remove-unused-variables
	@autopep8 api/ tools/python/ -i --recursive -a -a -a  --experimental \
		--select=E9,E2,E3,E5,F63,F7,F82,F4,F841,W291,W292 \
		--exclude .git,__pycache__,docs/source/conf.py,old,build,dist,venv \
		--max-line-length=140
	@echo "$@: OK"

After that I am down to this manageable number of code style violations:

22    E501 line too long (157 > 140 characters)
10    E999 SyntaxError: invalid syntax
36    F401 'swagger_server.models.api_parameter.ApiParameter' imported but unused
1     F403 'from swagger_server.models import *' used; unable to detect undefined names
1     F405 'ApiAsset' may be undefined, or defined from star imports: swagger_server.models
21    F841 local variable 'dictionary' is assigned to but never used
11    W291 trailing whitespace

With the auto-fix we could also include the generated API code in the linting, and I could add it to the generate_code script later on to not repeatedly have the violations reappear.