8000 ENH: Lint checks for PR diffs by ganesh-k13 · Pull Request #18423 · numpy/numpy · GitHub
[go: up one dir, main page]

Skip to content

ENH: Lint checks for PR diffs #18423

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Mar 7, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 29 additions & 11 deletions .github/workflows/build_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,24 @@ env:
PYTHON_VERSION: 3.7

jobs:
lint:
if: "github.repository == 'numpy/numpy' && !contains(github.event.head_commit.message, '[ci skip]') && !contains(github.event.head_commit.message, '[skip ci]') && !contains(github.event.head_commit.message, '[skip github]')"
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
with:
submodules: recursive
fetch-depth: 0
- uses: actions/setup-python@v2
with:
python-version: ${{ env.PYTHON_VERSION }}
- name: Install linter requirements
run:
python -m pip install -r linter_requirements.txt
- name: Run linter on PR diff
run:
python tools/linter.py --branch origin/${{ github.base_ref }}

smoke_test:
if: "github.repository == 'numpy/numpy' && !contains(github.event.head_commit.message, '[ci skip]') && !contains(github.event.head_commit.message, '[skip ci]') && !contains(github.event.head_commit.message, '[skip github]')"
runs-on: ubuntu-latest
Expand All @@ -33,7 +51,7 @@ jobs:
- uses: ./.github/actions

basic:
needs: smoke_test
needs: [smoke_test, lint]
runs-on: ubuntu-latest
strategy:
matrix:
Expand All @@ -49,7 +67,7 @@ jobs:
- uses: ./.github/actions

debug:
needs: smoke_test
needs: [smoke_test, lint]
runs-on: ubuntu-20.04
env:
USE_DEBUG: 1
Expand All @@ -64,7 +82,7 @@ jobs:
- uses: ./.github/actions

blas64:
needs: smoke_test
needs: [smoke_test, lint]
runs-on: ubuntu-latest
env:
NPY_USE_BLAS_ILP64: 1
Expand All @@ -79,7 +97,7 @@ jobs:
- uses: ./.github/actions

full:
needs: smoke_test
needs: [smoke_test, lint]
runs-on: ubuntu-18.04
env:
USE_WHEEL: 1
Expand All @@ -97,7 +115,7 @@ jobs:
- uses: ./.github/actions

benchmark:
needs: smoke_test
needs: [smoke_test, lint]
runs-on: ubuntu-latest
env:
PYTHONOPTIMIZE: 2
Expand All @@ -118,7 +136,7 @@ jobs:
- uses: ./.github/actions

no_relaxed_strides:
needs: smoke_test
needs: [smoke_test, lint]
runs-on: ubuntu-latest
env:
NPY_RELAXED_STRIDES_CHECKING: 0
Expand All @@ -135,7 +153,7 @@ jobs:
- uses: ./.github/actions

use_wheel:
needs: smoke_test
needs: [smoke_test, lint]
runs-on: ubuntu-latest
env:
USE_WHEEL: 1
Expand All @@ -151,7 +169,7 @@ jobs:
- uses: ./.github/actions

no_array_func:
needs: smoke_test
needs: [smoke_test, lint]
runs-on: ubuntu-latest
env:
NUMPY_EXPERIMENTAL_ARRAY_FUNCTION: 0
Expand All @@ -166,7 +184,7 @@ jobs:
- uses: ./.github/actions

no_openblas:
needs: smoke_test
needs: [smoke_test, lint]
runs-on: ubuntu-latest
env:
BLAS: None
Expand All @@ -184,7 +202,7 @@ jobs:
- uses: ./.github/actions

pypy37:
needs: smoke_test
needs: [smoke_test, lint]
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
Expand All @@ -197,7 +215,7 @@ jobs:
- uses: ./.github/actions

sdist:
needs: smoke_test
needs: [smoke_test, lint]
runs-on: ubuntu-latest
env:< 10000 /span>
USE_SDIST: 1
Expand Down
18 changes: 18 additions & 0 deletions azure-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,24 @@ stages:
- stage: InitialTests
jobs:

- job: Lint
condition: and(succeeded(), ne(variables['Build.SourceBranch'], 'refs/heads/master')) # skip for PR merges
pool:
vmImage: 'ubuntu-18.04'
steps:
- task: UsePythonVersion@0
inputs:
versionSpec: '3.8'
addToPath: true
architecture: 'x64'
- script: >-
python -m pip install -r linter_requirements.txt
displayName: 'Install tools'
failOnStderr: true
- script: |
python tools/linter.py --branch origin/$(System.PullRequest.TargetBranch)
displayName: 'Run Lint Checks'
failOnStderr: true
# Native build is based on gcc flag `-march=native`
- job: Linux_baseline_native
pool:
Expand Down
2 changes: 2 additions & 0 deletions linter_requirements.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
pycodestyle==2.5.0
GitPython==3.1.13
31 changes: 31 additions & 0 deletions runtests.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@
$ python runtests.py --gcov [...other args...]
$ python runtests.py --lcov-html

Run lint checks.
Provide target branch name or `uncommitted` to check before committing:

$ python runtests.py --lint main
$ python runtests.py --lint uncommitted

"""
#
# This is a generic test runner script for projects using NumPy's test
Expand Down Expand Up @@ -84,6 +90,10 @@ def main(argv):
parser.add_argument("--coverage", action="store_true", default=False,
help=("report coverage of project code. HTML output goes "
"under build/coverage"))
parser.add_argument("--lint", default=None,
help="'<Target Branch>' or 'uncommitted', passed to "
"tools/linter.py [--branch BRANCH] "
"[--uncommitted]")
parser.add_argument("--durations", action="store", default=-1, type=int,
help=("Time N slowest tests, time all if 0, time none if < 0"))
parser.add_argument("--gcov", action="store_true", default=False,
Expand Down Expand Up @@ -162,6 +172,9 @@ def main(argv):
print("*** Benchmarks should not be run against debug "
"version; remove -g flag ***")

if args.lint:
check_lint(args.lint)

if not args.no_build:
# we need the noarch path in case the package is pure python.
site_dir, site_dir_noarch = build_project(args)
Expand Down Expand Up @@ -637,6 +650,24 @@ def lcov_generate():
else:
print("HTML output generated under build/lcov/")

def check_lint(lint_args):
"""
Adds ROOT_DIR to path and performs lint checks.
This functions exits the program with status code of lint check.
"""
sys.path.append(ROOT_DIR)
try:
from tools.linter import DiffLinter
except ModuleNotFoundError as e:
print(f"Error: {e.msg}. "
"Install using linter_requirements.txt.")
sys.exit(1)

uncommitted = lint_args == "uncommitted"
branch = "main" if uncommitted else lint_args

DiffLinter(branch).run_lint(uncommitted)


if __name__ == "__main__":
main(argv=sys.argv[1:])
5 changes: 5 additions & 0 deletions tools/lint_diff.ini
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[pycodestyle]
max_line_length = 79
statistics = True
ignore = E121,E122,E123,E125,E126,E127,E128,E226,E251,E265,E266,E302,E402,E712,E721,E731,E741,W291,W293,W391,W503,W504
exclude = numpy/__config__.py
71 changes: 71 additions & 0 deletions tools/linter.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import os
import sys
import subprocess
from argparse import ArgumentParser
from git import Repo, exc

CONFIG = os.path.join(
os.path.abspath(os.path.dirname(__file__)),
'lint_diff.ini',
)


class DiffLinter:
def __init__(self, branch):
self.branch = branch
self.repo = Repo('.')
self.head = self.repo.head.commit

def get_branch_diff(self, uncommitted = False):
"""
Determine the first common ancestor commit.
Find diff between branch and FCA commit.
Note: if `uncommitted` is set, check only
uncommitted changes
"""
try:
commit = self.repo.merge_base(self.branch, self.head)[0]
except exc.GitCommandError:
print(f"Branch with name `{self.branch}` does not exist")
sys.exit(1)

if uncommitted:
diff = self.repo.git.diff(self.head, '--unified=0', '***.py')
else:
diff = self.repo.git.diff(commit, self.head,
'--unified=0', '***.py')
return diff

def run_pycodestyle(self, diff):
"""
Original Author: Josh Wilson (@person142)
Source:
https://github.com/scipy/scipy/blob/master/tools/lint_diff.py
Run pycodestyle on the given diff.
"""
res = subprocess.run(
['pycodestyle', '--diff', '--config', CONFIG],
input=diff,
stdout=subprocess.PIPE,
encoding='utf-8',
)
return res.returncode, res.stdout

def run_lint(self, uncommitted):
diff = self.get_branch_diff(uncommitted)
retcode, errors = self.run_pycodestyle(diff)

errors and print(errors)

sys.exit(retcode)


if __name__ == '__main__':
parser = ArgumentParser()
parser.add_argument("--branch", type=str, default='main',
help="The branch to diff against")
parser.add_argument("--uncommitted", action='store_true',
help="Check only uncommitted changes")
args = parser.parse_args()

DiffLinter(args.branch).run_lint(args.uncommitted)
0