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,160 @@ jobs: | |
| parameters: | ||
| name: WindowsPy27 | ||
| vmImage: vs2017-win2016 | ||
|
|
||
| - job: 'Checks_and_doc' | ||
| pool: | ||
| vmImage: ubuntu-16.04 | ||
| timeoutInMinutes: 90 | ||
| steps: | ||
| - script: | | ||
| # XXX not sure why setting $PATH here does not work | ||
| # echo '##vso[task.setvariable variable=PATH]$HOME/miniconda3/bin:$PATH' | ||
| 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 | ||
|
There was a problem hiding this comment. 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". There was a problem hiding this comment. 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 |
||
|
|
||
| - 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 | ||
| export NAME="master" | ||
| else | ||
| export NAME=$(System.PullRequest.PullRequestId) | ||
| fi | ||
| az extension add --name storage-preview | ||
| az storage blob upload-batch --connection-string $CONNECTION_STRING \ | ||
|
There was a problem hiding this comment. A difference I see is that I'm relying on - script: |
az extension add --name storage-preview
az storage blob upload-batch -s some/path/ -d '$web'
env:
AZURE_STORAGE_CONNECTION_STRING: $(CONNECTION_STRING)But I think this may be a red herring... will reply with another comment on what I think is going on. There was a problem hiding this comment. FYI @datapythonista the correct environment variable is now called There was a problem hiding this comment. (In the process of writing out the below instructions, I just realized what's happening. Will follow up with another comment.) Since the pipeline variable now has the correct name, this should work: - script: |
if [ "$(Build.SourceBranch)" == "refs/heads/master" ]; then
export NAME="master"
else
export NAME=$(System.PullRequest.PullRequestId)
fi
az extension add --name storage-preview
az storage blob upload-batch --source $SOURCE \
--destination '$web'
--destination-path "docs/$NAME"
displayName: 'Publishing docs (Azure storage)'
condition: true
env:
AZURE_STORAGE_CONNECTION_STRING: $(AZURE_STORAGE_CONNECTION_STRING) # this should map in the secret explicitly
SOURCE: $(Build.SourcesDirectory)/doc/build/html/ |
||
| --source $SOURCE \ | ||
| --destination $DESTINATION \ | ||
| --destination-path $NAME | ||
| echo "Documentation uploaded to https://pandas.blob.core.windows.net/docs/$NAME" | ||
| displayName: 'Publishing docs (Azure storage)' | ||
| condition: true | ||
| env: | ||
| CONNECTION_STRING: $(AZURE_STORAGE_CONNECTION_STRING) | ||
| SOURCE: $(Build.SourcesDirectory)/doc/build/html/ | ||
| DESTINATION: 'docs' | ||
|
There was a problem hiding this comment. I realize we haven't gotten to the bottom of the secrets/connection string thing, but also, the Azure static sites feature requires that your destination container be the string |
||
|
|
||
| - script: | | ||
| echo "Publishing documentation to GitHub pages is still done in Travis" | ||
| exit 0 | ||
| if [ "$(Build.SourceBranch)" == "refs/heads/master" ]; then | ||
| mkdir -p $REPO_DIR | ||
| rm -rf $REPO_DIR/* | ||
| cp -r doc/build/html/* $REPO_DIR/ | ||
| cd $REPO_DIR | ||
| git config --global user.email "[email protected]" | ||
| git config --global user.name "pandas-docs-bot" | ||
| git init | ||
| touch README | ||
| git add README | ||
| git commit -m "Initial commit" --allow-empty | ||
| git branch gh-pages | ||
| git checkout gh-pages | ||
| touch .nojekyll | ||
| git add --all . | ||
| git commit -m "Version" --allow-empty | ||
| git remote remove origin | ||
| git remote add origin "https://${TOKEN}@github.com/pandas-dev/pandas-docs-travis.git" | ||
| git fetch origin | ||
| git remote -v | ||
| git push origin gh-pages -f | ||
| fi | ||
| displayName: 'Publishing docs (GitHub pages)' | ||
| condition: true | ||
| env: | ||
| REPO_DIR: $(Build.ArtifactStagingDirectory)/pandas-docs-travis | ||
| TOKEN: $(GITHUB_DOCS_TOKEN) | ||
|
|
||
| - 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 | ||
|
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 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 | ||
|
There was a problem hiding this comment. Thanks @h-vetinari, that was added later, and I didn't see it after merging from master. Well spotted, I fixed it. |
||
| 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.
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.
FWIW, you can
echo "##vso[task.prependpath]$HOME/miniconda3/bin"here to make the path change durable for the job.