From 3cd6ac8e5d335559647b1701348414a07e40b1b5 Mon Sep 17 00:00:00 2001 From: Harutaka Kawamura Date: Mon, 8 Nov 2021 13:16:43 +0900 Subject: [PATCH 1/2] Clean up pylint command (#5014) Signed-off-by: harupy --- dev/set_matrix.py | 3 +-- dev/show_package_release_dates.py | 2 +- lint.sh | 41 +------------------------------ pylintrc | 15 ++++++++--- 4 files changed, 15 insertions(+), 46 deletions(-) diff --git a/dev/set_matrix.py b/dev/set_matrix.py index 62190857b1fda..5ac284524e27b 100644 --- a/dev/set_matrix.py +++ b/dev/set_matrix.py @@ -316,8 +316,7 @@ def process_requirements(requirements, version=None): ) if match_all: return packages - else: - return [] + return [] raise TypeError("Invalid object type for `requirements`: '{}'".format(type(requirements))) diff --git a/dev/show_package_release_dates.py b/dev/show_package_release_dates.py index 44792ba9c34cc..76b653792d4e9 100644 --- a/dev/show_package_release_dates.py +++ b/dev/show_package_release_dates.py @@ -6,7 +6,7 @@ def get_distributions(): - res = subprocess.run(["pip", "list"], stdout=subprocess.PIPE) + res = subprocess.run(["pip", "list"], stdout=subprocess.PIPE, check=True) pip_list_stdout = res.stdout.decode("utf-8") # `pip_list_stdout` looks like this: # `````````````````````````````````````````````````````````` diff --git a/lint.sh b/lint.sh index 41233b3ecb9b7..8ef4115b03044 100755 --- a/lint.sh +++ b/lint.sh @@ -5,30 +5,6 @@ err=0 trap 'err=1' ERR -FWDIR="$(cd "`dirname $0`"; pwd)" -cd "$FWDIR" - -# https://stackoverflow.com/a/17841619 -function join { - local d=$1 - shift - echo -n "$1" - shift - printf "%s" "${@/#/$d}" -} - -include_dirs=( - "mlflow" - "tests" -) - -exclude_dirs=( - "mlflow/protos" - "mlflow/server/js" - "mlflow/store/db_migrations" - "mlflow/temporary_db_migrations_for_pre_1_users" -) - echo -e "\n========== black ==========\n" # Exclude proto files because they are auto-generated black --check . @@ -38,23 +14,8 @@ if [ $? -ne 0 ]; then echo '$ pip install $(cat dev/lint-requirements.txt | grep "black==") && black .' fi -echo -e "\n========== pycodestyle ==========\n" -exclude=$(join "," "${exclude_dirs[@]}") -include=$(join " " "${include_dirs[@]}") -pycodestyle --max-line-length=100 --ignore=E203,W503 --exclude=$exclude -- $include - echo -e "\n========== pylint ==========\n" -# pylint's `--ignore` option filters files based on their base names, not paths. -# see: http://pylint.pycqa.org/en/latest/user_guide/run.html#command-line-options -# This behavior might cause us to unintentionally ignore some files. -# To avoid this issue, select files to lint using `git ls-files` and `grep`. -# Another advantage of this approach is we can apply pylint to all python scripts -# without creating `__init__.py` in all directories. -exclude="^\($(join "\|" "${exclude_dirs[@]}")\)/.\+\.py$" -include="^\($(join "\|" "${include_dirs[@]}")\)/.\+\.py$" -msg_template="{path} ({line},{column}): [{msg_id} {symbol}] {msg}" - -pylint --jobs=0 --msg-template="$msg_template" --rcfile="$FWDIR/pylintrc" $(git ls-files | grep $include | grep -v $exclude) +pylint $(git ls-files | grep '\.py$') echo -e "\n========== rstcheck ==========\n" rstcheck README.rst diff --git a/pylintrc b/pylintrc index 62cb9c6ec12ff..d237ff5ed29ed 100644 --- a/pylintrc +++ b/pylintrc @@ -7,19 +7,28 @@ extension-pkg-whitelist= # Add files or directories to the denylist. They should be base names, not # paths. -ignore=build,protos,sdk,db_migrations,temporary_db_migrations_for_pre_1_users - +ignore= # Add files or directories matching the regex patterns to the denylist. The # regex matches against base names, not paths. ignore-patterns= +# Add files or directories matching the regex patterns to the ignore-list. +# The regex matches against paths. +ignore-paths=setup.py, + docs, + examples, + mlflow/protos, + mlflow/server/js, + mlflow/store/db_migrations, + mlflow/temporary_db_migrations_for_pre_1_users + # Python code to execute, usually for sys.path manipulation such as # pygtk.require(). #init-hook= # Use multiple processes to speed up Pylint. -jobs=1 +jobs=0 # List of plugins (as comma separated values of python modules names) to load, # usually to register additional checkers. From 6af26f8d4b5db3c73e5fc258eaf3578d9e9a5236 Mon Sep 17 00:00:00 2001 From: Harutaka Kawamura Date: Tue, 9 Nov 2021 00:27:30 +0900 Subject: [PATCH 2/2] Add a pre-commit hook for running `black` and `pylint` (#5019) * add pre-commit hook Signed-off-by: harupy * address comment Signed-off-by: harupy * update comment Signed-off-by: harupy --- CONTRIBUTING.rst | 5 +++-- hooks/pre-commit | 8 ++++++++ 2 files changed, 11 insertions(+), 2 deletions(-) create mode 100755 hooks/pre-commit diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst index 1d3d7c2701955..d6ea7781c9932 100644 --- a/CONTRIBUTING.rst +++ b/CONTRIBUTING.rst @@ -125,8 +125,9 @@ you can `sign your work`_ when committing code changes and opening pull requests git config --global user.name "Your Name" git config --global user.email yourname@example.com -For convenience, we provide a pre-commit git hook that validates that commits are signed-off. -Enable it by running: +For convenience, we provide a pre-commit git hook that validates that commits are signed-off and +runs `black --check` and `pylint` to ensure the code will pass the lint check for python. +You can enable it by running: .. code-block:: bash diff --git a/hooks/pre-commit b/hooks/pre-commit new file mode 100755 index 0000000000000..eafa73c590cd4 --- /dev/null +++ b/hooks/pre-commit @@ -0,0 +1,8 @@ +set -ex + +diff=$(git diff --cached --name-only | grep '\.py$' || true) + +if [ ! -z "$diff" ]; then + black --check $diff + pylint $diff +fi