From c68854b516f0f6855260a71cd692f1890ffd8784 Mon Sep 17 00:00:00 2001 From: Dmitry Shemetov Date: Wed, 25 Oct 2023 10:49:46 -0700 Subject: [PATCH 1/3] feat+ci+lint: add incremental linting * add `inv lint` command to tasks.py * add `lint` workflow to .github/workflows/lint.yaml * update README.md with linting instructions --- .github/workflows/ci.yaml | 2 +- .github/workflows/lint.yaml | 29 +++++++++++++++++++++ .gitignore | 2 ++ README.md | 39 ++++++++++++++++++++++++++++ pyproject.toml | 51 ++++++++++++++++++++++--------------- requirements.lint.txt | 6 +++++ tasks.py | 30 +++++++++++++++++++--- 7 files changed, 135 insertions(+), 24 deletions(-) create mode 100644 .github/workflows/lint.yaml create mode 100644 requirements.lint.txt diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index fd2d5fb4b..b46787e49 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -65,7 +65,7 @@ jobs: docker network create --driver bridge delphi-net docker run --rm -d -p 13306:3306 --network delphi-net --name delphi_database_epidata --cap-add=sys_nice delphi_database_epidata docker run --rm -d -p 6379:6379 --network delphi-net --env "REDIS_PASSWORD=1234" --name delphi_redis delphi_redis - + - run: | wget https://raw.githubusercontent.com/eficode/wait-for/master/wait-for diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml new file mode 100644 index 000000000..78d2cdb3d --- /dev/null +++ b/.github/workflows/lint.yaml @@ -0,0 +1,29 @@ +name: Lint + +on: + pull_request: + branches: + - dev + +jobs: + lint: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + with: + fetch-depth: 0 + - uses: actions/setup-python@v4 + with: + python-version: "3.8" + cache: "pip" + cache-dependency-path: "requirements.lint.txt" + - name: Install dependencies + run: | + python -m pip install --upgrade pip + pip install -r requirements.lint.txt + - name: Commit Range + id: commit-range + uses: akaihola/darker/.github/actions/commit-range@1.7.1 + - name: Run lint + run: | + inv lint --revision=${{ steps.commit-range.outputs.commit-range }} diff --git a/.gitignore b/.gitignore index e8e3d4303..d9894ea28 100644 --- a/.gitignore +++ b/.gitignore @@ -43,3 +43,5 @@ __pycache__/ rsconnect/ *.Rproj +venv +env diff --git a/README.md b/README.md index 170c9a4ef..3ce42dc85 100644 --- a/README.md +++ b/README.md @@ -64,6 +64,45 @@ $ cd repos $ pip install -e . --config-settings editable_mode=strict ``` +## Linting and Formatting + +The command `inv lint` will lint your changes (using +[lint-diffs](https://github.com/AtakamaLLC/lint-diffs/)) and `inv format` will +format your changes (using [darker](https://github.com/akaihola/darker)). This +will allow us to incrementally bring this repo up to PEP8 compliance. There is +[a CI action](.github/workflows/lint.yaml) to ensure that all new code is +compliant. + +To setup the linter commands locally, run the following commands in the root of +the repository: + +```sh +# Create a virtual environment +python -m venv venv +source venv/bin/activate +# Install lint dependencies +pip install -r requirements.lint.txt +# Run lint +inv lint +# Run formatter (will tell you which files it modified) +inv format +``` + +If you get the error `ERROR:darker.git:fatal: Not a valid commit name `, +then it's likely because your local main branch is not up to date; either you +need to rebase or merge. Note that `darker` reads from `pyproject.toml` for +default settings. + +If the lines you change are in a file that uses 2 space indentation, `darker` +will indent the lines around your changes and not the rest, which will likely +break the code; in that case, you should probably just pass the whole file +through black. You can do that with the following command (using the same +virtual environment as above): + +```sh +env/bin/black +``` + # COVIDcast At the present, our primary focus is developing and expanding the diff --git a/pyproject.toml b/pyproject.toml index a4399ca9b..5240a2259 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,26 +1,37 @@ [tool.black] -line-length = 100 +line-length = 120 target-version = ['py38'] -include = 'server,tests/server' + +[tool.darker] +revision = 'origin/dev...' +color = true +isort = true + +[tool.isort] +profile = "black" +known_third_party = ["pytest"] [tool.pylint] - [tool.pylint.'MESSAGES CONTROL'] - max-line-length = 100 - disable = [ - 'logging-format-interpolation', - # Allow pytest functions to be part of a class - 'no-self-use', - 'too-many-locals', - 'too-many-arguments', - # Allow pytest classes to have one test - 'too-few-public-methods', - ] +[tool.pylint.main] +max-line-length = 120 +disable = [ + 'logging-format-interpolation', + # Allow pytest functions to be part of a class + 'no-self-use', + 'too-many-locals', + 'too-many-arguments', + 'too-many-branches', + 'too-many-statements', + # Allow pytest classes to have one test + 'too-few-public-methods', +] +enable = 'useless-suppression' - [tool.pylint.'BASIC'] - # Allow arbitrarily short-named variables. - variable-rgx = ['[a-z_][a-z0-9_]*'] - argument-rgx = [ '[a-z_][a-z0-9_]*' ] - attr-rgx = ['[a-z_][a-z0-9_]*'] +[tool.pylint.basic] +# Allow arbitrarily short-named variables. +variable-rgx = '[A-Za-z_][a-z0-9_]*' +argument-rgx = '[A-Za-z_][a-z0-9_]*' +attr-rgx = '[A-Za-z_][a-z0-9_]*' - [tool.pylint.'DESIGN'] - ignored-argument-names = ['(_.*|run_as_module)'] +[tool.pylint.design] +ignored-argument-names = ['(_.*|run_as_module)'] diff --git a/requirements.lint.txt b/requirements.lint.txt new file mode 100644 index 000000000..17db2a4af --- /dev/null +++ b/requirements.lint.txt @@ -0,0 +1,6 @@ +# Keep versions in sync with cmu-delphi/covidcast-indicators +darker[isort]~=2.1.1 +invoke>=1.4.1 +lint_diffs==0.1.22 +pylint==2.8.3 +requests \ No newline at end of file diff --git a/tasks.py b/tasks.py index fd7115f7e..fe3752639 100644 --- a/tasks.py +++ b/tasks.py @@ -1,3 +1,8 @@ +"""Repo tasks.""" + +import pathlib + +import requests from invoke import task @@ -12,9 +17,6 @@ def update_gdoc( sources_url="https://docs.google.com/spreadsheets/d/e/2PACX-1vRfXo-qePhrYGAoZqewVnS1kt9tfnUTLgtkV7a-1q7yg4FoZk0NNGuB1H6k10ah1Xz5B8l1S1RB17N6/pub?gid=0&single=true&output=csv", signal_url="https://docs.google.com/spreadsheets/d/e/2PACX-1vRfXo-qePhrYGAoZqewVnS1kt9tfnUTLgtkV7a-1q7yg4FoZk0NNGuB1H6k10ah1Xz5B8l1S1RB17N6/pub?gid=329338228&single=true&output=csv", ): - import requests - import pathlib - base_dir = pathlib.Path("./src/server/endpoints/covidcast_utils/") def _migrate_file(url: str, filename: str): @@ -26,3 +28,25 @@ def _migrate_file(url: str, filename: str): _migrate_file(sources_url, "db_sources.csv") _migrate_file(signal_url, "db_signals.csv") + + +@task +def lint(c, revision="origin/dev"): + """Lint. + + Linters automatically read from `pyproject.toml`. + """ + c.run(f"darker --diff --check --revision {revision} .", warn=True) + c.run("echo '\n'") + out = c.run(f"git diff -U0 {revision} | lint-diffs") + if out.stdout.strip() != "=== pylint: mine=0, always=0": + print(out.stdout) + + +@task +def format(c, revision="origin/dev"): # pylint: disable=redefined-builtin + """Format. + + Darker will format files for you and print which ones changed. + """ + c.run(f"darker --verbose --revision {revision} .", warn=True) From 8061bd04551afd9432b488873cf2a7bd6126f5de Mon Sep 17 00:00:00 2001 From: Dmitry Shemetov Date: Thu, 6 Jun 2024 21:48:40 -0700 Subject: [PATCH 2/3] test: linter error --- src/client/delphi_epidata.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/delphi_epidata.py b/src/client/delphi_epidata.py index 6fb7ab1ef..04ad334df 100644 --- a/src/client/delphi_epidata.py +++ b/src/client/delphi_epidata.py @@ -72,7 +72,7 @@ def _request_with_retry(endpoint, params={}): """Make request with a retry if an exception is thrown.""" request_url = f"{Epidata.BASE_URL}/{endpoint}/" if Epidata.debug: - Epidata.logger.info("Sending GET request", url=request_url, params=params, headers=_HEADERS, auth=Epidata.auth) + Epidata.logger.info("Sending GET request", url=request_url, params=params, headers=_HEADERS, auth=Epidata.auth) # introduce a very long line comment if Epidata.sandbox: resp = requests.Response() resp._content = b'true' From 92849e4ad51915c95792fe6c4bf01c3de264a8e2 Mon Sep 17 00:00:00 2001 From: Dmitry Shemetov Date: Thu, 6 Jun 2024 21:49:24 -0700 Subject: [PATCH 3/3] test: fix linter error --- src/client/delphi_epidata.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/delphi_epidata.py b/src/client/delphi_epidata.py index 04ad334df..6fb7ab1ef 100644 --- a/src/client/delphi_epidata.py +++ b/src/client/delphi_epidata.py @@ -72,7 +72,7 @@ def _request_with_retry(endpoint, params={}): """Make request with a retry if an exception is thrown.""" request_url = f"{Epidata.BASE_URL}/{endpoint}/" if Epidata.debug: - Epidata.logger.info("Sending GET request", url=request_url, params=params, headers=_HEADERS, auth=Epidata.auth) # introduce a very long line comment + Epidata.logger.info("Sending GET request", url=request_url, params=params, headers=_HEADERS, auth=Epidata.auth) if Epidata.sandbox: resp = requests.Response() resp._content = b'true'