From 881a9b58897b02e90bd05a2f3583cbc55c58dafa Mon Sep 17 00:00:00 2001 From: 01-vyom Date: Tue, 22 Jun 2021 14:16:12 +0530 Subject: [PATCH 1/9] STYLE: moving spacing conventions in cython casting to pre-commit --- .pre-commit-config.yaml | 3 +++ ci/code_checks.sh | 17 ----------------- 2 files changed, 3 insertions(+), 17 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index d580fcf4fc545..0ec59d17175f1 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -102,6 +102,9 @@ repos: # Incorrect code-block / IPython directives |\.\.\ code-block\ :: |\.\.\ ipython\ :: + + # Cython casting is of the form `obj` as opposed to ` obj` + |[a-zA-Z0-9*]> types_or: [python, cython, rst] - id: pip-to-conda name: Generate pip dependency from conda diff --git a/ci/code_checks.sh b/ci/code_checks.sh index 59548ecd3c710..e502d50040237 100755 --- a/ci/code_checks.sh +++ b/ci/code_checks.sh @@ -41,23 +41,6 @@ if [[ "$GITHUB_ACTIONS" == "true" ]]; then INVGREP_PREPEND="##[error]" fi -### LINTING ### -if [[ -z "$CHECK" || "$CHECK" == "lint" ]]; then - - # Check that cython casting is of the form `obj` as opposed to ` obj`; - # it doesn't make a difference, but we want to be internally consistent. - # Note: this grep pattern is (intended to be) equivalent to the python - # regex r'(?])> ' - MSG='Linting .pyx code for spacing conventions in casting' ; echo $MSG - invgrep -r -E --include '*.pyx' --include '*.pxi.in' '[a-zA-Z0-9*]> ' pandas/_libs - RET=$(($RET + $?)) ; echo $MSG "DONE" - - # readability/casting: Warnings about C casting instead of C++ casting - # runtime/int: Warnings about using C number types instead of C++ ones - # build/include_subdir: Warnings about prefacing included header files with directory - -fi - ### PATTERNS ### if [[ -z "$CHECK" || "$CHECK" == "patterns" ]]; then From 2f49b8d32dc579358c63341730854f856ceb4315 Mon Sep 17 00:00:00 2001 From: 01-vyom Date: Tue, 27 Jul 2021 00:18:24 +0530 Subject: [PATCH 2/9] BUG: Fix pre-commit hook for cython casting check --- .pre-commit-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 0ec59d17175f1..58654d29c5afd 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -104,7 +104,7 @@ repos: |\.\.\ ipython\ :: # Cython casting is of the form `obj` as opposed to ` obj` - |[a-zA-Z0-9*]> + |[a-zA-Z0-9*]>\s types_or: [python, cython, rst] - id: pip-to-conda name: Generate pip dependency from conda From 62d46c4e2c8ce4ddb28f8cf60de60406f1467ae7 Mon Sep 17 00:00:00 2001 From: 01-vyom Date: Mon, 2 Aug 2021 20:32:41 +0530 Subject: [PATCH 3/9] ENH: Add new hook for cython casting --- .pre-commit-config.yaml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 58654d29c5afd..ffbe5154d84ed 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -102,10 +102,14 @@ repos: # Incorrect code-block / IPython directives |\.\.\ code-block\ :: |\.\.\ ipython\ :: - + types_or: [python, cython, rst] + - id: cython-casting + name: Cython Casting + language: pygrep + entry: | # Cython casting is of the form `obj` as opposed to ` obj` |[a-zA-Z0-9*]>\s - types_or: [python, cython, rst] + types: [cython] - id: pip-to-conda name: Generate pip dependency from conda description: This hook checks if the conda environment.yml and requirements-dev.txt are equal From 39caae31fd2c708a7fd5936e2acd8d103a45080b Mon Sep 17 00:00:00 2001 From: 01-vyom Date: Tue, 3 Aug 2021 11:48:10 +0530 Subject: [PATCH 4/9] ENH: Changed hook for cython casting --- .pre-commit-config.yaml | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index ffbe5154d84ed..1dccea1d9cff2 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -9,11 +9,11 @@ repos: - id: absolufy-imports files: ^pandas/ - repo: https://github.com/python/black - rev: 21.5b2 + rev: 21.7b0 hooks: - id: black - repo: https://github.com/codespell-project/codespell - rev: v2.0.0 + rev: v2.1.0 hooks: - id: codespell types_or: [python, rst, markdown] @@ -53,16 +53,16 @@ repos: types: [text] args: [--append-config=flake8/cython-template.cfg] - repo: https://github.com/PyCQA/isort - rev: 5.8.0 + rev: 5.9.3 hooks: - id: isort - repo: https://github.com/asottile/pyupgrade - rev: v2.18.3 + rev: v2.23.1 hooks: - id: pyupgrade args: [--py37-plus] - repo: https://github.com/pre-commit/pygrep-hooks - rev: v1.8.0 + rev: v1.9.0 hooks: - id: rst-backticks - id: rst-directive-colons @@ -106,10 +106,8 @@ repos: - id: cython-casting name: Cython Casting language: pygrep - entry: | - # Cython casting is of the form `obj` as opposed to ` obj` - |[a-zA-Z0-9*]>\s - types: [cython] + entry: '[a-zA-Z0-9*]> ' + files: (\.pyx|\.pxi.in)$ - id: pip-to-conda name: Generate pip dependency from conda description: This hook checks if the conda environment.yml and requirements-dev.txt are equal From be94fc46420775b618e1709e0de6b5ffee1f2544 Mon Sep 17 00:00:00 2001 From: Marco Edward Gorelli Date: Tue, 3 Aug 2021 08:30:42 +0100 Subject: [PATCH 5/9] Update .pre-commit-config.yaml --- .pre-commit-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index aa2d30214f0b1..1555d1720a753 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -104,7 +104,7 @@ repos: |\.\.\ ipython\ :: types_or: [python, cython, rst] - id: cython-casting - name: Cython Casting + name: Check Cython casting is `obj`, not ` obj` language: pygrep entry: '[a-zA-Z0-9*]> ' files: (\.pyx|\.pxi.in)$ From d7b61ae948f6cacb0ca4c9ade173fa790dab96a2 Mon Sep 17 00:00:00 2001 From: Marco Edward Gorelli Date: Tue, 3 Aug 2021 08:31:35 +0100 Subject: [PATCH 6/9] Update code_checks.sh --- ci/code_checks.sh | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/ci/code_checks.sh b/ci/code_checks.sh index 1d567da52c13d..3c5557defb45f 100755 --- a/ci/code_checks.sh +++ b/ci/code_checks.sh @@ -10,15 +10,14 @@ # # Usage: # $ ./ci/code_checks.sh # run all checks -# $ ./ci/code_checks.sh lint # run linting only # $ ./ci/code_checks.sh patterns # check for patterns that should not exist # $ ./ci/code_checks.sh code # checks on imported code # $ ./ci/code_checks.sh doctests # run doctests # $ ./ci/code_checks.sh docstrings # validate docstring errors # $ ./ci/code_checks.sh typing # run static type analysis -[[ -z "$1" || "$1" == "lint" || "$1" == "patterns" || "$1" == "code" || "$1" == "doctests" || "$1" == "docstrings" || "$1" == "typing" ]] || \ - { echo "Unknown command $1. Usage: $0 [lint|patterns|code|doctests|docstrings|typing]"; exit 9999; } +[[ -z "$1" || "$1" == "patterns" || "$1" == "code" || "$1" == "doctests" || "$1" == "docstrings" || "$1" == "typing" ]] || \ + { echo "Unknown command $1. Usage: $0 [patterns|code|doctests|docstrings|typing]"; exit 9999; } BASE_DIR="$(dirname $0)/.." RET=0 From 0ded6bbb794601482f2a00b0a456d1ce6d846eb9 Mon Sep 17 00:00:00 2001 From: 01-vyom Date: Tue, 10 Aug 2021 00:34:54 +0530 Subject: [PATCH 7/9] ENH: Removed lint comments from code checks --- ci/code_checks.sh | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/ci/code_checks.sh b/ci/code_checks.sh index 200c06bac97e0..1c96702d08bf4 100755 --- a/ci/code_checks.sh +++ b/ci/code_checks.sh @@ -3,21 +3,20 @@ # Run checks related to code quality. # # This script is intended for both the CI and to check locally that code standards are -# respected. We are currently linting (PEP-8 and similar), looking for patterns of +# respected. We are currently looking for patterns of # common mistakes (sphinx directives with missing blank lines, old style classes, # unwanted imports...), we run doctests here (currently some files only), and we # validate formatting error in docstrings. # # Usage: # $ ./ci/code_checks.sh # run all checks -# $ ./ci/code_checks.sh lint # run linting only # $ ./ci/code_checks.sh code # checks on imported code # $ ./ci/code_checks.sh doctests # run doctests # $ ./ci/code_checks.sh docstrings # validate docstring errors # $ ./ci/code_checks.sh typing # run static type analysis -[[ -z "$1" || "$1" == "lint" || "$1" == "code" || "$1" == "doctests" || "$1" == "docstrings" || "$1" == "typing" ]] || \ - { echo "Unknown command $1. Usage: $0 [lint|code|doctests|docstrings|typing]"; exit 9999; } +[[ -z "$1" || "$1" == "code" || "$1" == "doctests" || "$1" == "docstrings" || "$1" == "typing" ]] || \ + { echo "Unknown command $1. Usage: $0 [code|doctests|docstrings|typing]"; exit 9999; } BASE_DIR="$(dirname $0)/.." RET=0 From 06bb18f35287e08b9f13cc89de7912f326d3a40f Mon Sep 17 00:00:00 2001 From: 01-vyom Date: Tue, 10 Aug 2021 14:30:55 +0530 Subject: [PATCH 8/9] DOC: Changed documentation for code check --- .github/workflows/ci.yml | 4 ---- ci/code_checks.sh | 4 +--- doc/source/development/contributing_codebase.rst | 8 +++----- 3 files changed, 4 insertions(+), 12 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b9fe5461aff37..e7a26a7905799 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -48,10 +48,6 @@ jobs: - name: Build Pandas uses: ./.github/actions/build_pandas - - name: Linting - run: ci/code_checks.sh lint - if: always() - - name: Checks on imported code run: ci/code_checks.sh code if: always() diff --git a/ci/code_checks.sh b/ci/code_checks.sh index 1c96702d08bf4..f9860de3b2bb3 100755 --- a/ci/code_checks.sh +++ b/ci/code_checks.sh @@ -3,9 +3,7 @@ # Run checks related to code quality. # # This script is intended for both the CI and to check locally that code standards are -# respected. We are currently looking for patterns of -# common mistakes (sphinx directives with missing blank lines, old style classes, -# unwanted imports...), we run doctests here (currently some files only), and we +# respected. We run doctests here (currently some files only), and we # validate formatting error in docstrings. # # Usage: diff --git a/doc/source/development/contributing_codebase.rst b/doc/source/development/contributing_codebase.rst index 721b1af126709..a1f45c314738e 100644 --- a/doc/source/development/contributing_codebase.rst +++ b/doc/source/development/contributing_codebase.rst @@ -23,11 +23,9 @@ contributing them to the project:: ./ci/code_checks.sh -The script verifies the linting of code files, it looks for common mistake patterns -(like missing spaces around sphinx directives that make the documentation not -being rendered properly) and it also validates the doctests. It is possible to -run the checks independently by using the parameters ``lint``, ``patterns`` and -``doctests`` (e.g. ``./ci/code_checks.sh lint``). +The script validates the doctests and formatting error in docstrings. It is possible to +run the checks independently by using the parameters ``docstring``, and +``doctests`` (e.g. ``./ci/code_checks.sh doctests``). In addition, because a lot of people use our library, it is important that we do not make sudden changes to the code that could have the potential to break From b58bbe3a1b4cdf567d3e447d92c4ef6547fcfabe Mon Sep 17 00:00:00 2001 From: Marco Edward Gorelli Date: Tue, 10 Aug 2021 10:10:28 +0100 Subject: [PATCH 9/9] mention imports and typing in doc --- doc/source/development/contributing_codebase.rst | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/doc/source/development/contributing_codebase.rst b/doc/source/development/contributing_codebase.rst index a1f45c314738e..53e66eeff51e6 100644 --- a/doc/source/development/contributing_codebase.rst +++ b/doc/source/development/contributing_codebase.rst @@ -23,9 +23,10 @@ contributing them to the project:: ./ci/code_checks.sh -The script validates the doctests and formatting error in docstrings. It is possible to -run the checks independently by using the parameters ``docstring``, and -``doctests`` (e.g. ``./ci/code_checks.sh doctests``). +The script validates the doctests, formatting in docstrings, static typing, and +imported modules. It is possible to run the checks independently by using the +parameters ``docstring``, ``code``, ``typing``, and ``doctests`` +(e.g. ``./ci/code_checks.sh doctests``). In addition, because a lot of people use our library, it is important that we do not make sudden changes to the code that could have the potential to break