Skip to content

Commit

Permalink
Used flake8 + no formatter instead of pep8 + pylint + yapf. (tensorfl…
Browse files Browse the repository at this point in the history
…ow#904)

* Used flake8 instead of pep8 + pylintrc. Removed formatting.
* Added a separate build for flake8
  • Loading branch information
gabrieldemarmiesse authored and seanpmorgan committed Jan 25, 2020
1 parent fa8e966 commit 01bef16
Show file tree
Hide file tree
Showing 14 changed files with 58 additions and 740 deletions.
32 changes: 32 additions & 0 deletions .flake8
@@ -0,0 +1,32 @@
[flake8]

ignore =
# defaults flake8 ignores
E121,E123,E126,E226,E24,E704,W503,W504
# Function name should be lowercase
N802
# lowercase ... imported as non lowercase
# Useful to ignore for "import keras.backend as K"
N812
# TODO: remove all the exceptions below
# do not use bare 'except'
E722
# Imported but unused
F401
# the backslash is redundant between brackets
E502
# do not assign a lambda expression, use a def
E731
# local variable is assigned to but never used
F841
# trailing whitespace
W291
# expected 2 blank lines after class or function definition, found 1
E305


#imported but unused in __init__.py, that's ok.
per-file-ignores = **/__init__.py:F401

# TODO: reduce the max line length to 90-ish
max-line-length = 210
10 changes: 10 additions & 0 deletions .github/workflows/ci_test.yml
Expand Up @@ -66,3 +66,13 @@ jobs:
run: |
set -x -e
bash tools/run_docker.sh -c 'make sanity-check'
flake8-test:
name: Flake8
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v1
- uses: actions/setup-python@v1
- name: install flake8
run: pip install flake8
- name: Run flake8
run: flake8
2 changes: 2 additions & 0 deletions .gitignore
Expand Up @@ -3,6 +3,7 @@

# Include travis.yml
\!.travis.yml
\!.flake8

# Byte-compiled / optimized / DLL files
__pycache__/
Expand Down Expand Up @@ -42,5 +43,6 @@ wheels/
/.bazelrc
/bazel-*
/artifacts
.bazelrc

buildifier.*
24 changes: 2 additions & 22 deletions STYLE_GUIDE.md
Expand Up @@ -17,28 +17,8 @@ void formatted_code_again;
#### Python
Python code should conform to [PEP8](https://www.python.org/dev/peps/pep-0008/).

Addons uses [yapf](https://github.com/google/yapf) to format code,
and [pylint](https://www.pylint.org/) for code analysis.
You can disable them locally like this:

```python
# yapf: disable
FOO = {
# ... some very large, complex data literal.
}

BAR = [
# ... another large data literal.
]
# yapf: enable
```

```python
# pylint: disable=protected-access
foo._protected_member
# pylint: enable=protected-access
```

Addons uses [flake8](http://flake8.pycqa.org/en/latest/) to check pep8 compliance and
code analysis.

#### TensorFlow Conventions

Expand Down
2 changes: 1 addition & 1 deletion build_deps/toolchains/gpu/find_cuda_config.py
Expand Up @@ -152,7 +152,7 @@ def _get_ld_config_paths():
def _get_default_cuda_paths(cuda_version):
if not cuda_version:
cuda_version = "*"
elif not "." in cuda_version:
elif "." not in cuda_version:
cuda_version = cuda_version + ".*"

if _is_windows():
Expand Down
2 changes: 1 addition & 1 deletion tensorflow_addons/losses/focal_loss_test.py
@@ -1,4 +1,4 @@
## Copyright 2019 The TensorFlow Authors. All Rights Reserved.
# Copyright 2019 The TensorFlow Authors. All Rights Reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
Expand Down
3 changes: 1 addition & 2 deletions tensorflow_addons/metrics/cohens_kappa.py
Expand Up @@ -150,8 +150,7 @@ def result(self):
pred_ratings_hist = tf.reduce_sum(self.conf_mtx, axis=0)

# 4. Get the outer product
out_prod = pred_ratings_hist[..., None] * \
actual_ratings_hist[None, ...]
out_prod = pred_ratings_hist[..., None] * actual_ratings_hist[None, ...]

# 5. Normalize the confusion matrix and outer product
conf_mtx = self.conf_mtx / tf.reduce_sum(self.conf_mtx)
Expand Down
4 changes: 2 additions & 2 deletions tensorflow_addons/optimizers/conditional_gradient_test.py
Expand Up @@ -119,7 +119,7 @@ def testVariablesAcrossGraphs(self):
optimizer.minimize(loss, var_list=[var0, var1])
optimizer_variables = optimizer.variables()
# There should be three items. The first item is iteration,
#and one item for each variable.
# and one item for each variable.
self.assertStartsWith(optimizer_variables[1].name,
"ConditionalGradient/var0")
self.assertStartsWith(optimizer_variables[2].name,
Expand Down Expand Up @@ -159,7 +159,7 @@ def loss():
pred = tf.matmul(tf.nn.embedding_lookup([var0], [0]), x)
return pred * pred

#the gradient based on the current loss function
# the gradient based on the current loss function
grads0_0 = 32 * 1.0 + 40 * 2.0
grads0_1 = 40 * 1.0 + 50 * 2.0
grads0 = tf.constant([[grads0_0, grads0_1]], dtype=dtype)
Expand Down
172 changes: 4 additions & 168 deletions tools/ci_build/ci_sanity.sh
Expand Up @@ -14,11 +14,10 @@
# limitations under the License.
# ==============================================================================
#
# Usage: ci_sanity.sh [--pep8] [--incremental] [bazel flags]
# Usage: ci_sanity.sh [--incremental] [bazel flags]
#
# Options:
# run sanity checks: python 2&3 pylint checks and bazel nobuild
# --pep8 run pep8 test only
# --incremental Performs checks incrementally, by using the files changed in
# the latest commit

Expand All @@ -32,165 +31,6 @@ if [[ ! -d "tensorflow_addons" ]]; then
exit 1
fi

# Run pylint
do_pylint() {
# Usage: do_pylint [--incremental]
#
# Options:
# --incremental Performs check on only the python files changed in the
# last non-merge git commit.

# Use this list to whitelist pylint errors
ERROR_WHITELIST=""

echo "ERROR_WHITELIST=\"${ERROR_WHITELIST}\""

if [[ $# != "0" ]] && [[ $# != "1" ]]; then
echo "Invalid syntax when invoking do_pylint"
echo "Usage: do_pylint [--incremental]"
return 1
fi

PYLINT_BIN="python3 -m pylint"

PYTHON_SRC_FILES=$(get_py_files_to_check $1)
if [[ -z ${PYTHON_SRC_FILES} ]]; then
echo "do_pylint found no Python files to check. Returning."
return 0
fi

PYLINTRC_FILE="${SCRIPT_DIR}/pylintrc"

if [[ ! -f "${PYLINTRC_FILE}" ]]; then
die "ERROR: Cannot find pylint rc file at ${PYLINTRC_FILE}"
fi

NUM_SRC_FILES=$(echo ${PYTHON_SRC_FILES} | wc -w)
NUM_CPUS=$(num_cpus)

echo "Running pylint on ${NUM_SRC_FILES} files with ${NUM_CPUS} "\
"parallel jobs..."
echo ""

PYLINT_START_TIME=$(date +'%s')
OUTPUT_FILE="$(mktemp)_pylint_output.log"
ERRORS_FILE="$(mktemp)_pylint_errors.log"
NONWL_ERRORS_FILE="$(mktemp)_pylint_nonwl_errors.log"

rm -rf ${OUTPUT_FILE}
rm -rf ${ERRORS_FILE}
rm -rf ${NONWL_ERRORS_FILE}
touch ${NONWL_ERRORS_FILE}

${PYLINT_BIN} --rcfile="${PYLINTRC_FILE}" --output-format=parseable \
--jobs=${NUM_CPUS} ${PYTHON_SRC_FILES} > ${OUTPUT_FILE} 2>&1
PYLINT_END_TIME=$(date +'%s')

echo ""
echo "pylint took $((PYLINT_END_TIME - PYLINT_START_TIME)) s"
echo ""

# Report only what we care about
# Ref https://pylint.readthedocs.io/en/latest/technical_reference/features.html
# E: all errors
# W0311 bad-indentation
# W0312 mixed-indentation
# C0330 bad-continuation
# C0301 line-too-long
# C0326 bad-whitespace
# W0611 unused-import
# W0622 redefined-builtin
grep -E '(\[E|\[W0311|\[W0312|\[C0330|\[C0301|\[C0326|\[W0611|\[W0622)' ${OUTPUT_FILE} > ${ERRORS_FILE}

N_ERRORS=0
while read -r LINE; do
IS_WHITELISTED=0
for WL_REGEX in ${ERROR_WHITELIST}; do
if echo ${LINE} | grep -q "${WL_REGEX}"; then
echo "Found a whitelisted error:"
echo " ${LINE}"
IS_WHITELISTED=1
fi
done

if [[ ${IS_WHITELISTED} == "0" ]]; then
echo "${LINE}" >> ${NONWL_ERRORS_FILE}
echo "" >> ${NONWL_ERRORS_FILE}
((N_ERRORS++))
fi
done <${ERRORS_FILE}

echo ""
if [[ ${N_ERRORS} != 0 ]]; then
echo "FAIL: Found ${N_ERRORS} non-whitelited pylint errors:"
cat "${NONWL_ERRORS_FILE}"
return 1
else
echo "PASS: No non-whitelisted pylint errors were found."
return 0
fi
}

# Run pep8 check
do_pep8() {
# Usage: do_pep8 [--incremental]
# Options:
# --incremental Performs check on only the python files changed in the
# last non-merge git commit.

PEP8_BIN="/usr/local/bin/pep8"
PEP8_CONFIG_FILE="${SCRIPT_DIR}/pep8"

if [[ "$1" == "--incremental" ]]; then
PYTHON_SRC_FILES=$(get_py_files_to_check --incremental)
NUM_PYTHON_SRC_FILES=$(echo ${PYTHON_SRC_FILES} | wc -w)

echo "do_pep8 will perform checks on only the ${NUM_PYTHON_SRC_FILES} "\
"Python file(s) changed in the last non-merge git commit due to the "\
"--incremental flag:"
echo "${PYTHON_SRC_FILES}"
echo ""
else
PYTHON_SRC_FILES=$(get_py_files_to_check)
fi

if [[ -z ${PYTHON_SRC_FILES} ]]; then
echo "do_pep8 found no Python files to check. Returning."
return 0
fi

if [[ ! -f "${PEP8_CONFIG_FILE}" ]]; then
die "ERROR: Cannot find pep8 config file at ${PEP8_CONFIG_FILE}"
fi
echo "See \"${PEP8_CONFIG_FILE}\" for pep8 config( e.g., ignored errors)"

NUM_SRC_FILES=$(echo ${PYTHON_SRC_FILES} | wc -w)

echo "Running pep8 on ${NUM_SRC_FILES} files"
echo ""

PEP8_START_TIME=$(date +'%s')
PEP8_OUTPUT_FILE="$(mktemp)_pep8_output.log"

rm -rf ${PEP8_OUTPUT_FILE}

${PEP8_BIN} --config="${PEP8_CONFIG_FILE}" --statistics \
${PYTHON_SRC_FILES} 2>&1 | tee ${PEP8_OUTPUT_FILE}
PEP8_END_TIME=$(date +'%s')

echo ""
echo "pep8 took $((PEP8_END_TIME - PEP8_START_TIME)) s"
echo ""

if [[ -s ${PEP8_OUTPUT_FILE} ]]; then
echo "FAIL: pep8 found above errors and/or warnings."
return 1
else
echo "PASS: No pep8 errors or warnings were found"
return 0
fi
}


#Check for the bazel cmd status (First arg is error message)
cmd_status(){
Expand Down Expand Up @@ -230,20 +70,16 @@ do_check_code_format_test() {
}

# Supply all sanity step commands and descriptions
SANITY_STEPS=("do_check_code_format_test" "do_pylint" "do_bazel_nobuild" "do_check_file_name_test")
SANITY_STEPS_DESC=("Check code style" "Python 3 pylint" "bazel nobuild" "Check file names for cases")
SANITY_STEPS=("do_check_code_format_test" "do_bazel_nobuild" "do_check_file_name_test")
SANITY_STEPS_DESC=("Check code style" "bazel nobuild" "Check file names for cases")

INCREMENTAL_FLAG=""
DEFAULT_BAZEL_CONFIGS=""

# Parse command-line arguments
BAZEL_FLAGS=${DEFAULT_BAZEL_CONFIGS}
for arg in "$@"; do
if [[ "${arg}" == "--pep8" ]]; then
# Only run pep8 test if "--pep8" option supplied
SANITY_STEPS=("do_pep8")
SANITY_STEPS_DESC=("pep8 test")
elif [[ "${arg}" == "--incremental" ]]; then
if [[ "${arg}" == "--incremental" ]]; then
INCREMENTAL_FLAG="--incremental"
else
BAZEL_FLAGS="${BAZEL_FLAGS} ${arg}"
Expand Down
39 changes: 2 additions & 37 deletions tools/ci_build/code_format.sh
Expand Up @@ -93,41 +93,6 @@ do_bazel_config_format_check() {
fi
}

do_python_format_check() {
PYTHON_SRC_FILES=$(get_py_files_to_check $INCREMENTAL_FLAG)
if [[ -z $PYTHON_SRC_FILES ]]; then
echo "do_python_format_check will NOT run due to"\
"the absence of code changes."
return 0
fi

NUM_BUILD_FILES=$(echo ${PYTHON_SRC_FILES} | wc -w)
echo "Running do_python_format_check on ${NUM_BUILD_FILES} files"
echo ""

YAPFRC_FILE="${SCRIPT_DIR}/yapfrc"
if [[ ! -f "${YAPFRC_FILE}" ]]; then
die "ERROR: Cannot find yapf rc file at ${YAPFRC_FILE}"
fi
YAPF_OPTS="--style=$YAPFRC_FILE --parallel"

if [[ ! -z $IN_PLACE_FLAG ]]; then
echo "Auto format..."
yapf $YAPF_OPTS --in-place --verbose $PYTHON_SRC_FILES
docformatter --wrap-summaries 79 --wrap-descriptions 72 --in-place $PYTHON_SRC_FILES
fi

UNFORMATTED_CODES=$(yapf $YAPF_OPTS --diff $PYTHON_SRC_FILES)
if [[ $? != "0" || ! -z "$UNFORMATTED_CODES" ]]; then
echo "Find unformatted codes:"
echo "$UNFORMATTED_CODES"
echo "Python format check fails."
return 1
else
echo "Python format check success."
return 0
fi
}

do_clang_format_check() {
CLANG_SRC_FILES=$(get_clang_files_to_check $INCREMENTAL_FLAG)
Expand Down Expand Up @@ -168,8 +133,8 @@ do_clang_format_check() {
}

# Supply all auto format step commands and descriptions
FORMAT_STEPS=("do_bazel_config_format_check" "do_python_format_check" "do_clang_format_check")
FORMAT_STEPS_DESC=("Check Bazel file format" "Check python file format" "Check C++ file format")
FORMAT_STEPS=("do_bazel_config_format_check" "do_clang_format_check")
FORMAT_STEPS_DESC=("Check Bazel file format" "Check C++ file format")

FAIL_COUNTER=0
PASS_COUNTER=0
Expand Down
3 changes: 1 addition & 2 deletions tools/ci_build/install/ci_requirements.txt
@@ -1,3 +1,2 @@
pylint == 2.3.1
flake8==3.7.9
docformatter == 1.1
yapf == 0.26.0

0 comments on commit 01bef16

Please sign in to comment.