CI: Linting with azure instead of travis #22854
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| | @@ -23,3 +23,127 @@ jobs: | |
| parameters: | ||
| name: WindowsPy27 | ||
| vmImage: vs2017-win2016 | ||
| | ||
| - job: 'Checks_and_doc' | ||
| pool: | ||
| vmImage: ubuntu-16.04 | ||
| timeoutInMinutes: 90 | ||
| steps: | ||
| - script: | | ||
| # XXX next command should avoid redefining the path in every step, but | ||
| # made the process crash as it couldn't find deactivate | ||
| #echo '##vso[task.prependpath]$HOME/miniconda3/bin' | ||
| echo '##vso[task.setvariable variable=CONDA_ENV]pandas-dev' | ||
| echo '##vso[task.setvariable variable=ENV_FILE]environment.yml' | ||
| echo '##vso[task.setvariable variable=AZURE]true' | ||
| displayName: 'Setting environment variables' | ||
| | ||
| # Do not require a conda environment | ||
| - script: | | ||
| export PATH=$HOME/miniconda3/bin:$PATH | ||
| ci/code_checks.sh patterns | ||
| displayName: 'Looking for unwanted patterns' | ||
| condition: true | ||
| | ||
| - script: | | ||
| export PATH=$HOME/miniconda3/bin:$PATH | ||
| sudo apt-get install -y libc6-dev-i386 | ||
| ci/incremental/install_miniconda.sh | ||
| ci/incremental/setup_conda_environment.sh | ||
| displayName: 'Set up environment' | ||
| | ||
| # Do not require pandas | ||
| - script: | | ||
| export PATH=$HOME/miniconda3/bin:$PATH | ||
| source activate pandas-dev | ||
| ci/code_checks.sh lint | ||
| displayName: 'Linting' | ||
| condition: true | ||
| | ||
| - script: | | ||
| export PATH=$HOME/miniconda3/bin:$PATH | ||
| source activate pandas-dev | ||
| ci/code_checks.sh dependencies | ||
| displayName: 'Dependencies consistency' | ||
| condition: true | ||
| | ||
| - script: | | ||
| export PATH=$HOME/miniconda3/bin:$PATH | ||
| source activate pandas-dev | ||
| ci/incremental/build.sh | ||
| displayName: 'Build' | ||
| condition: true | ||
| | ||
| # Require pandas | ||
| - script: | | ||
| export PATH=$HOME/miniconda3/bin:$PATH | ||
| source activate pandas-dev | ||
| ci/code_checks.sh code | ||
| displayName: 'Checks on imported code' | ||
| condition: true | ||
| | ||
| - script: | | ||
| export PATH=$HOME/miniconda3/bin:$PATH | ||
| source activate pandas-dev | ||
| ci/code_checks.sh doctests | ||
| displayName: 'Running doctests' | ||
| condition: true | ||
| | ||
| - script: | | ||
| export PATH=$HOME/miniconda3/bin:$PATH | ||
| source activate pandas-dev | ||
| ci/code_checks.sh docstrings | ||
| displayName: 'Docstring validation' | ||
| condition: true | ||
| | ||
| - script: | | ||
| export PATH=$HOME/miniconda3/bin:$PATH | ||
| source activate pandas-dev | ||
| pytest --capture=no --strict scripts | ||
| displayName: 'Testing docstring validaton script' | ||
| condition: true | ||
| | ||
| - script: | | ||
| export PATH=$HOME/miniconda3/bin:$PATH | ||
| source activate pandas-dev | ||
| doc/make.py html | ||
| displayName: 'Building docs' | ||
| condition: true | ||
| | ||
| - script: | | ||
| if [ "$(Build.SourceBranch)" == "refs/heads/master" ]; then | ||
| az extension add --name storage-preview | ||
| az storage blob upload-batch --connection-string $CONNECTION_STRING \ | ||
| --source $SOURCE \ | ||
| --destination '$web' | ||
| echo "Documentation uploaded to https://pandas.blob.core.windows.net" | ||
| else | ||
| echo "Documentation is only uploaded for commits to master, and not PRs" | ||
| fi | ||
| displayName: 'Publishing docs (Azure storage)' | ||
| condition: true | ||
| env: | ||
| CONNECTION_STRING: $(AZURE_STORAGE_CONNECTION_STRING) | ||
| SOURCE: $(Build.SourcesDirectory)/doc/build/html/ | ||
| | ||
| - script: | | ||
| export PATH=$HOME/miniconda3/bin:$PATH | ||
| source activate pandas-dev | ||
| git remote add upstream https://github.com/pandas-dev/pandas.git | ||
| git fetch upstream | ||
| if git diff upstream/master --name-only | grep -q "^asv_bench/"; then | ||
| cd asv_bench | ||
| asv machine --yes | ||
| ASV_OUTPUT="$(asv dev)" | ||
| if [[ $(echo "$ASV_OUTPUT" | grep "failed") ]]; then | ||
| echo "##vso[task.logissue type=error]Benchmarks run with errors" | ||
| echo $ASV_OUTPUT | ||
| exit 1 | ||
| else | ||
| echo "Benchmarks run without errors" | ||
| fi | ||
| else | ||
| echo "Benchmarks did not run, no changes detected" | ||
| fi | ||
| displayName: 'Running benchmarks' | ||
| condition: true | ||
| Member There was a problem hiding this comment. Why did you add running benchmarks when the original issue was just to migrate Also, as your changes did not actually modify any Python benchmark code, it was not possible to review how it would look in the Azure logs. This log here from another PR (#23752) is unfortunately impossible to read: https://dev.azure.com/pandas-dev/pandas/_build/results?buildId=4580 Member There was a problem hiding this comment. Also, it's somewhat debatable as to whether we even want to just run benchmarks when there's only a diff in | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| | @@ -5,25 +5,48 @@ | |
| # 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 | ||
| # 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 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 dependencies # check that dependencies are consistent | ||
| | ||
| [[ -z "$1" || "$1" == "lint" || "$1" == "patterns" || "$1" == "code" || "$1" == "doctests" || "$1" == "docstrings" || "$1" == "dependencies" ]] || \ | ||
| { echo "Unknown command $1. Usage: $0 [lint|patterns|code|doctests|docstrings|dependencies]"; exit 9999; } | ||
| | ||
| BASE_DIR="$(dirname $0)/.." | ||
| RET=0 | ||
| CHECK=$1 | ||
| | ||
| function invgrep { | ||
| # grep with inverse exist status and formatting for azure-pipelines | ||
| # | ||
| # This function works exactly as grep, but with opposite exit status: | ||
| # - 0 (success) when no patterns are found | ||
| # - 1 (fail) when the patterns are found | ||
| # | ||
| # This is useful for the CI, as we want to fail if one of the patterns | ||
| # that we want to avoid is found by grep. | ||
| if [[ "$AZURE" == "true" ]]; then | ||
| set -o pipefail | ||
| grep -n "$@" | awk -F ":" '{print "##vso[task.logissue type=error;sourcepath=" $1 ";linenumber=" $2 ";] Found unwanted pattern: " $3}' | ||
| else | ||
| grep "$@" | ||
| fi | ||
| return $((! $?)) | ||
| } | ||
| | ||
| if [[ "$AZURE" == "true" ]]; then | ||
| FLAKE8_FORMAT="##vso[task.logissue type=error;sourcepath=%(path)s;linenumber=%(row)s;columnnumber=%(col)s;code=%(code)s;]%(text)s" | ||
| else | ||
| FLAKE8_FORMAT="default" | ||
| fi | ||
| | ||
| ### LINTING ### | ||
| if [[ -z "$CHECK" || "$CHECK" == "lint" ]]; then | ||
| | @@ -35,30 +58,30 @@ if [[ -z "$CHECK" || "$CHECK" == "lint" ]]; then | |
| | ||
| # pandas/_libs/src is C code, so no need to search there. | ||
| MSG='Linting .py code' ; echo $MSG | ||
| flake8 --format="$FLAKE8_FORMAT" . | ||
| RET=$(($RET + $?)) ; echo $MSG "DONE" | ||
| | ||
| MSG='Linting .pyx code' ; echo $MSG | ||
| flake8 --format="$FLAKE8_FORMAT" pandas --filename=*.pyx --select=E501,E302,E203,E111,E114,E221,E303,E128,E231,E126,E265,E305,E301,E127,E261,E271,E129,W291,E222,E241,E123,F403,C400,C401,C402,C403,C404,C405,C406,C407,C408,C409,C410,C411 | ||
| RET=$(($RET + $?)) ; echo $MSG "DONE" | ||
| | ||
| MSG='Linting .pxd and .pxi.in' ; echo $MSG | ||
| flake8 --format="$FLAKE8_FORMAT" pandas/_libs --filename=*.pxi.in,*.pxd --select=E501,E302,E203,E111,E114,E221,E303,E231,E126,F403 | ||
| RET=$(($RET + $?)) ; echo $MSG "DONE" | ||
| | ||
| echo "flake8-rst --version" | ||
| flake8-rst --version | ||
| | ||
| MSG='Linting code-blocks in .rst documentation' ; echo $MSG | ||
| flake8-rst doc/source --filename=*.rst --format="$FLAKE8_FORMAT" | ||
| RET=$(($RET + $?)) ; echo $MSG "DONE" | ||
| | ||
| # Check that cython casting is of the form `<type>obj` as opposed to `<type> 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 | ||
| | @@ -88,43 +111,48 @@ if [[ -z "$CHECK" || "$CHECK" == "patterns" ]]; then | |
| | ||
| # Check for imports from pandas.core.common instead of `import pandas.core.common as com` | ||
| MSG='Check for non-standard imports' ; echo $MSG | ||
| invgrep -R --include="*.py*" -E "from pandas.core.common import " pandas | ||
| RET=$(($RET + $?)) ; echo $MSG "DONE" | ||
| | ||
| MSG='Check for pytest warns' ; echo $MSG | ||
| invgrep -r -E --include '*.py' 'pytest\.warns' pandas/tests/ | ||
| RET=$(($RET + $?)) ; echo $MSG "DONE" | ||
| | ||
| # Check for the following code in testing: `np.testing` and `np.array_equal` | ||
| MSG='Check for invalid testing' ; echo $MSG | ||
| invgrep -r -E --include '*.py' --exclude testing.py '(numpy|np)(\.testing|\.array_equal)' pandas/tests/ | ||
| RET=$(($RET + $?)) ; echo $MSG "DONE" | ||
| | ||
| # Check for the following code in the extension array base tests: `tm.assert_frame_equal` and `tm.assert_series_equal` | ||
| MSG='Check for invalid EA testing' ; echo $MSG | ||
| invgrep -r -E --include '*.py' --exclude base.py 'tm.assert_(series|frame)_equal' pandas/tests/extension/base | ||
| RET=$(($RET + $?)) ; echo $MSG "DONE" | ||
| | ||
| MSG='Check for deprecated messages without sphinx directive' ; echo $MSG | ||
| invgrep -R --include="*.py" --include="*.pyx" -E "(DEPRECATED|DEPRECATE|Deprecated)(:|,|\.)" pandas | ||
| RET=$(($RET + $?)) ; echo $MSG "DONE" | ||
| | ||
| MSG='Check for old-style classes' ; echo $MSG | ||
| invgrep -R --include="*.py" -E "class\s\S*[^)]:" pandas scripts | ||
| RET=$(($RET + $?)) ; echo $MSG "DONE" | ||
| | ||
| MSG='Check for backticks incorrectly rendering because of missing spaces' ; echo $MSG | ||
| invgrep -R --include="*.rst" -E "[a-zA-Z0-9]\`\`?[a-zA-Z0-9]" doc/source/ | ||
| RET=$(($RET + $?)) ; echo $MSG "DONE" | ||
| | ||
| MSG='Check for incorrect sphinx directives' ; echo $MSG | ||
| invgrep -R --include="*.py" --include="*.pyx" --include="*.rst" -E "\.\. (autosummary|contents|currentmodule|deprecated|function|image|important|include|ipython|literalinclude|math|module|note|raw|seealso|toctree|versionadded|versionchanged|warning):[^:]" ./pandas ./doc/source | ||
| RET=$(($RET + $?)) ; echo $MSG "DONE" | ||
| | ||
| MSG='Check that the deprecated `assert_raises_regex` is not used (`pytest.raises(match=pattern)` should be used instead)' ; echo $MSG | ||
| ! grep -R --exclude=*.pyc --exclude=testing.py --exclude=test_testing.py assert_raises_regex pandas | ||
| ||
| RET=$(($RET + $?)) ; echo $MSG "DONE" | ||
| | ||
| fi | ||
| | ||
| ### CODE ### | ||
| if [[ -z "$CHECK" || "$CHECK" == "code" ]]; then | ||
| | ||
| MSG='Check for modules that pandas should not import' ; echo $MSG | ||
| python -c " | ||
| import sys | ||
| | @@ -135,7 +163,7 @@ blacklist = {'bs4', 'gcsfs', 'html5lib', 'ipython', 'jinja2' 'hypothesis', | |
| 'tables', 'xlrd', 'xlsxwriter', 'xlwt'} | ||
| mods = blacklist & set(m.split('.')[0] for m in sys.modules) | ||
| if mods: | ||
| sys.stderr.write('err: pandas should not import: {}\n'.format(', '.join(mods))) | ||
| sys.exit(len(mods)) | ||
| " | ||
| RET=$(($RET + $?)) ; echo $MSG "DONE" | ||
| | @@ -157,7 +185,7 @@ if [[ -z "$CHECK" || "$CHECK" == "doctests" ]]; then | |
| | ||
| MSG='Doctests generic.py' ; echo $MSG | ||
| pytest -q --doctest-modules pandas/core/generic.py \ | ||
| -k"-_set_axis_name -_xs -describe -droplevel -groupby -interpolate -pct_change -pipe -reindex -reindex_axis -to_json -transpose -values -xs -to_clipboard" | ||
| RET=$(($RET + $?)) ; echo $MSG "DONE" | ||
| | ||
| MSG='Doctests top-level reshaping functions' ; echo $MSG | ||
| | @@ -178,11 +206,22 @@ if [[ -z "$CHECK" || "$CHECK" == "doctests" ]]; then | |
| | ||
| fi | ||
| | ||
| ### DOCSTRINGS ### | ||
| if [[ -z "$CHECK" || "$CHECK" == "docstrings" ]]; then | ||
| | ||
| MSG='Validate docstrings (GL06, SS04, PR03, PR05, EX04)' ; echo $MSG | ||
| $BASE_DIR/scripts/validate_docstrings.py --format=azure --errors=GL06,SS04,PR03,PR05,EX04 | ||
| RET=$(($RET + $?)) ; echo $MSG "DONE" | ||
| | ||
| fi | ||
| | ||
| ### DEPENDENCIES ### | ||
| if [[ -z "$CHECK" || "$CHECK" == "dependencies" ]]; then | ||
| | ||
| MSG='Check that requirements-dev.txt has been generated from environment.yml' ; echo $MSG | ||
| $BASE_DIR/scripts/generate_pip_deps_from_conda.py --compare --azure | ||
| RET=$(($RET + $?)) ; echo $MSG "DONE" | ||
| | ||
| fi | ||
| | ||
| exit $RET | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm...IMO, I think running doctests should be a separate build entirely from linting. They are two semantically different things, and the separation would allow us to provide more semantically clear display names such as "Doctests" and "Linting" instead of the slightly vaguer mashup of "Checks_and_doc".
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see there was a concern about not having to use an extra build for this due to resource constraints (the conversation is a very long in this PR...)? However, I'm not sure I fully understand...
Also, based on the conversation, are the
masterdocs no longer being published on https://pandas-docs.github.io/pandas-docs-travis? In which case, we need to update theGITHUB_ISSUEtemplate that we have here.