Skip to content
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

Add ruff - Replace Black, isort and pyupgrade #2620

Merged
merged 22 commits into from
Jan 4, 2024
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
33 changes: 8 additions & 25 deletions .github/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,45 +35,28 @@ pip install -e .

## Code formatting

### Black
### Ruff

All Python code in nf-core/tools must be passed through the [Black Python code formatter](https://black.readthedocs.io/en/stable/).
All Python code in nf-core/tools must be passed through the [Ruff code linter and formatter](https://github.com/astral-sh/ruff).
This ensures a harmonised code formatting style throughout the package, from all contributors.

You can run Black on the command line (it's included in `requirements-dev.txt`) - eg. to run recursively on the whole repository:
You can run Ruff on the command line (it's included in `requirements-dev.txt`) - eg. to run recursively on the whole repository:

```bash
black .
ruff format .
```

Alternatively, Black has [integrations for most common editors](https://black.readthedocs.io/en/stable/editor_integration.html)
Alternatively, Ruff has [integrations for most common editors](https://github.com/astral-sh/ruff-lsp) and VSCode(https://github.com/astral-sh/ruff-vscode)
to automatically format code when you hit save.
You can also set it up to run when you [make a commit](https://black.readthedocs.io/en/stable/version_control_integration.html).

There is an automated CI check that runs when you open a pull-request to nf-core/tools that will fail if
any code does not adhere to Black formatting.
any code does not adhere to Ruff formatting.

### isort

All Python code must also be passed through [isort](https://pycqa.github.io/isort/index.html).
This ensures a harmonised imports throughout the package, from all contributors.

To run isort on the command line recursively on the whole repository you can use:

```bash
isort .
```

isort also has [plugins for most common editors](https://github.com/pycqa/isort/wiki/isort-Plugins)
to automatically format code when you hit save.
Or [version control integration](https://pycqa.github.io/isort/docs/configuration/pre-commit.html) to set it up to run when you make a commit.

There is an automated CI check that runs when you open a pull-request to nf-core/tools that will fail if
any code does not adhere to isort formatting.
Ruff has been adopted for linting and formatting in replacement of Black, isort (for imports) and pyupgrade. It also includes Flake8.

### pre-commit hooks

This repository comes with [pre-commit](https://pre-commit.com/) hooks for black, isort and Prettier. pre-commit automatically runs checks before a commit is committed into the git history. If all checks pass, the commit is made, if files are changed by the pre-commit hooks, the user is informed and has to stage the changes and attempt the commit again.
This repository comes with [pre-commit](https://pre-commit.com/) hooks for ruff and Prettier. pre-commit automatically runs checks before a commit is committed into the git history. If all checks pass, the commit is made, if files are changed by the pre-commit hooks, the user is informed and has to stage the changes and attempt the commit again.

You can use the pre-commit hooks if you like, but you don't have to. The CI on Github will run the same checks as the tools installed with pre-commit. If the pre-commit checks pass, then the same checks in the CI will pass, too.

Expand Down
9 changes: 9 additions & 0 deletions .github/workflows/create-lint-wf.yml
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ jobs:
- name: Install Prettier and editorconfig-checker
run: npm install -g prettier editorconfig-checker

- name: Install ruff
run: |
python -m pip install --upgrade pip
pip install ruff

# Build a pipeline from the template
- name: nf-core create
run: |
Expand All @@ -76,6 +81,10 @@ jobs:
- name: Run Prettier --check
run: prettier --check create-lint-wf/nf-core-testpipeline

- name: Run Ruff check
run: ruff check create-lint-wf/nf-core-testpipeline
- name: Run Ruff format
run: ruff format create-lint-wf/nf-core-testpipeline
- name: Run ECLint check
run: editorconfig-checker -exclude README.md $(find nf-core-testpipeline/.* -type f | grep -v '.git\|.py\|md\|json\|yml\|yaml\|html\|css\|work\|.nextflow\|build\|nf_core.egg-info\|log.txt\|Makefile')
working-directory: create-lint-wf
Expand Down
20 changes: 9 additions & 11 deletions .github/workflows/fix-linting.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,21 +34,19 @@ jobs:
- name: Run 'prettier --write'
run: prettier --write ${GITHUB_WORKSPACE}

- name: Run Black
uses: psf/black@stable
with:
# Override to remove the default --check flag so that we make changes
options: "--color"

- name: Set up Python 3.11
uses: actions/setup-python@v4
with:
python-version: 3.11
- name: python-isort
uses: isort/isort-action@v1.0.0
with:
isortVersion: "latest"
requirementsFiles: "requirements.txt requirements-dev.txt"

- name: Install Ruff
run: |
python -m pip install --upgrade pip
pip install ruff
- name: Run Ruff
run: |
ruff check --fix .
ruff format .

- name: Commit & push changes
run: |
Expand Down
44 changes: 20 additions & 24 deletions .github/workflows/lint-code.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,52 +44,48 @@ jobs:
- name: Run Prettier --check
run: prettier --check ${GITHUB_WORKSPACE}

PythonBlack:
Ruff:
runs-on: ["self-hosted"]
steps:
- uses: actions/checkout@v4
- name: Check out source-code repository
uses: actions/checkout@v4

- name: Set up Python 3.11
uses: actions/setup-python@v4
with:
python-version: 3.11
- name: Install Ruff
run: |
python -m pip install --upgrade pip
pip install ruff
- name: Run Ruff check
run: ruff check .
mirpedrol marked this conversation as resolved.
Show resolved Hide resolved

- name: Check code lints with Black
uses: psf/black@stable
- name: Run Ruff format
run: ruff format .

# If the above check failed, post a comment on the PR explaining the failure
- name: Post PR comment
if: failure()
uses: mshick/add-pr-comment@v1
with:
message: |
## Python linting (`black`) is failing
## Python linting (`ruff`) is failing

To keep the code consistent with lots of contributors, we run automated code consistency checks.
To fix this CI test, please run:

* Install [`black`](https://black.readthedocs.io/en/stable/): `pip install black`
* Fix formatting errors in your pipeline: `black .`
* Install [`ruff`](https://github.com/astral-sh/ruff): `pip install ruff`
* Fix formatting errors in your pipeline: `ruff check --fix .` and `ruff format .`

Once you push these changes the test should pass, and you can hide this comment :+1:

We highly recommend setting up Black in your code editor so that this formatting is done automatically on save. Ask about it on Slack for help!
We highly recommend setting up Ruff in your code editor so that this formatting is done automatically on save. Ask about it on Slack for help!

Thanks again for your contribution!
repo-token: ${{ secrets.GITHUB_TOKEN }}
allow-repeats: false

isort:
runs-on: ["self-hosted"]
steps:
- name: Check out source-code repository
uses: actions/checkout@v4

- name: Set up Python 3.11
uses: actions/setup-python@v4
with:
python-version: 3.11
- name: python-isort
uses: isort/isort-action@v1.1.0
with:
isortVersion: "latest"
requirementsFiles: "requirements.txt requirements-dev.txt"

static-type-check:
runs-on: ["self-hosted"]
steps:
Expand Down
1 change: 1 addition & 0 deletions .gitpod.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,4 @@ vscode:
# - nextflow.nextflow # Nextflow syntax highlighting
- oderwat.indent-rainbow # Highlight indentation level
- streetsidesoftware.code-spell-checker # Spelling checker for source code
- charliermarsh.ruff # Code linter Ruff
17 changes: 5 additions & 12 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -1,21 +1,14 @@
repos:
- repo: https://github.com/psf/black
rev: 23.1.0
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.1.9
hooks:
- id: black
- repo: https://github.com/pycqa/isort
rev: 5.12.0
hooks:
- id: isort
- id: ruff # linter
args: [--fix, --exit-non-zero-on-fix] # sort imports and fix
- id: ruff-format # formatter
- repo: https://github.com/pre-commit/mirrors-prettier
rev: "v2.7.1"
hooks:
- id: prettier
- repo: https://github.com/asottile/pyupgrade
rev: v3.15.0
hooks:
- id: pyupgrade
args: [--py38-plus]
- repo: https://github.com/pre-commit/mirrors-mypy
rev: "v1.7.1" # Use the sha / tag you want to point at
hooks:
Expand Down
4 changes: 4 additions & 0 deletions .prettierignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,7 @@ testing
nf_core/module-template/meta.yml
nf_core/module-template/tests/tags.yml
nf_core/subworkflow-template/tests/tags.yml
# don't run on things handled by ruff
*.py
*.pyc

2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

### General

- Add Ruff linter and formatter replacing Black, isort and pyupgrade ([#2620](https://github.com/nf-core/tools/pull/2620))

# [v2.11.1 - Magnesium Dragon Patch](https://github.com/nf-core/tools/releases/tag/2.11) - [2023-12-20]

### Template
Expand Down
3 changes: 1 addition & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@

[![Python tests](https://github.com/nf-core/tools/workflows/Python%20tests/badge.svg?branch=master&event=push)](https://github.com/nf-core/tools/actions?query=workflow%3A%22Python+tests%22+branch%3Amaster)
[![codecov](https://codecov.io/gh/nf-core/tools/branch/master/graph/badge.svg)](https://codecov.io/gh/nf-core/tools)
[![code style: black](https://img.shields.io/badge/code%20style-black-000000.svg)](https://github.com/psf/black)
[![code style: prettier](https://img.shields.io/badge/code%20style-prettier-ff69b4.svg)](https://github.com/prettier/prettier)
[![Imports: isort](https://img.shields.io/badge/%20imports-isort-%231674b1?style=flat&labelColor=ef8336)](https://pycqa.github.io/isort/)
[![code style: Ruff](https://img.shields.io/endpoint?url=https://raw.githubusercontent.com/charliermarsh/ruff/main/assets/badge/v1.json)](https://github.com/charliermarsh/ruff)

[![install with Bioconda](https://img.shields.io/badge/install%20with-bioconda-brightgreen.svg)](https://bioconda.github.io/recipes/nf-core/README.html)
[![install with PyPI](https://img.shields.io/badge/install%20with-PyPI-blue.svg)](https://pypi.org/project/nf-core/)
Expand Down
4 changes: 2 additions & 2 deletions docs/api/_src/conf.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
# -*- coding: utf-8 -*-
#
# Configuration file for the Sphinx documentation builder.
#
Expand All @@ -16,9 +15,10 @@
import sys
from typing import Dict

sys.path.insert(0, os.path.abspath("../../../nf_core"))
import nf_core

sys.path.insert(0, os.path.abspath("../../../nf_core"))

# -- Project information -----------------------------------------------------

project = "nf-core/tools"
Expand Down
16 changes: 8 additions & 8 deletions nf_core/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -934,7 +934,7 @@ def modules_lint(ctx, tool, dir, registry, key, all, fail_warned, local, passed,
Test modules within a pipeline or a clone of the
nf-core/modules repository.
"""
from nf_core.components.lint import LintException
from nf_core.components.lint import LintExceptionError
from nf_core.modules import ModuleLint

try:
Expand All @@ -960,7 +960,7 @@ def modules_lint(ctx, tool, dir, registry, key, all, fail_warned, local, passed,
)
if len(module_lint.failed) > 0:
sys.exit(1)
except LintException as e:
except LintExceptionError as e:
log.error(e)
sys.exit(1)
except (UserWarning, LookupError) as e:
Expand Down Expand Up @@ -1020,7 +1020,7 @@ def bump_versions(ctx, tool, dir, all, show_all):
the nf-core/modules repo.
"""
from nf_core.modules.bump_versions import ModuleVersionBumper
from nf_core.modules.modules_utils import ModuleException
from nf_core.modules.modules_utils import ModuleExceptionError

try:
version_bumper = ModuleVersionBumper(
Expand All @@ -1030,7 +1030,7 @@ def bump_versions(ctx, tool, dir, all, show_all):
ctx.obj["modules_repo_no_pull"],
)
version_bumper.bump_versions(module=tool, all_modules=all, show_uptodate=show_all)
except ModuleException as e:
except ModuleExceptionError as e:
log.error(e)
sys.exit(1)
except (UserWarning, LookupError) as e:
Expand Down Expand Up @@ -1207,7 +1207,7 @@ def subworkflows_lint(ctx, subworkflow, dir, registry, key, all, fail_warned, lo
Test subworkflows within a pipeline or a clone of the
nf-core/modules repository.
"""
from nf_core.components.lint import LintException
from nf_core.components.lint import LintExceptionError
from nf_core.subworkflows import SubworkflowLint

try:
Expand All @@ -1232,7 +1232,7 @@ def subworkflows_lint(ctx, subworkflow, dir, registry, key, all, fail_warned, lo
)
if len(subworkflow_lint.failed) > 0:
sys.exit(1)
except LintException as e:
except LintExceptionError as e:
log.error(e)
sys.exit(1)
except (UserWarning, LookupError) as e:
Expand Down Expand Up @@ -1647,7 +1647,7 @@ def sync(dir, from_branch, pull_request, github_repository, username, template_y
the pipeline. It is run automatically for all pipelines when ever a
new release of [link=https://github.com/nf-core/tools]nf-core/tools[/link] (and the included template) is made.
"""
from nf_core.sync import PipelineSync, PullRequestException, SyncException
from nf_core.sync import PipelineSync, PullRequestExceptionError, SyncExceptionError
from nf_core.utils import is_pipeline_directory

# Check if pipeline directory contains necessary files
Expand All @@ -1657,7 +1657,7 @@ def sync(dir, from_branch, pull_request, github_repository, username, template_y
sync_obj = PipelineSync(dir, from_branch, pull_request, github_repository, username, template_yaml)
try:
sync_obj.sync()
except (SyncException, PullRequestException) as e:
except (SyncExceptionError, PullRequestExceptionError) as e:
log.error(e)
sys.exit(1)

Expand Down
6 changes: 3 additions & 3 deletions nf_core/bump_version.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def bump_pipeline_version(pipeline_obj: Pipeline, new_version: str) -> None:
[
(
f"/releases/tag/{current_version}",
f"/tree/dev",
"/tree/dev",
)
],
)
Expand All @@ -78,7 +78,7 @@ def bump_pipeline_version(pipeline_obj: Pipeline, new_version: str) -> None:
pipeline_obj,
[
(
f"/tree/dev",
"/tree/dev",
f"/releases/tag/{multiqc_new_version}",
)
],
Expand Down Expand Up @@ -187,7 +187,7 @@ def update_file_version(filename: Union[str, Path], pipeline_obj: Pipeline, patt
fn = pipeline_obj._fp(filename)
content = ""
try:
with open(fn, "r") as fh:
with open(fn) as fh:
content = fh.read()
except FileNotFoundError:
log.warning(f"File not found: '{fn}'")
Expand Down
4 changes: 2 additions & 2 deletions nf_core/components/components_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ def check_patch_paths(self, patch_path: Path, module_name: str) -> None:
if patch_path.exists():
log.info(f"Modules {module_name} contains a patch file.")
rewrite = False
with open(patch_path, "r") as fh:
with open(patch_path) as fh:
lines = fh.readlines()
for index, line in enumerate(lines):
# Check if there are old paths in the patch file and replace
Expand Down Expand Up @@ -264,7 +264,7 @@ def check_if_in_include_stmts(self, component_path: str) -> Dict[str, List[Dict[
if self.repo_type == "pipeline":
workflow_files = Path(self.dir, "workflows").glob("*.nf")
for workflow_file in workflow_files:
with open(workflow_file, "r") as fh:
with open(workflow_file) as fh:
# Check if component path is in the file using mmap
with mmap.mmap(fh.fileno(), 0, access=mmap.ACCESS_READ) as s:
if s.find(component_path.encode()) != -1:
Expand Down
4 changes: 2 additions & 2 deletions nf_core/components/components_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def get_repo_info(directory: str, use_prompt: Optional[bool] = True) -> Tuple[st
raise UserWarning("Repository type could not be established")

# Check if it's a valid answer
if not repo_type in ["pipeline", "modules"]:
if repo_type not in ["pipeline", "modules"]:
raise UserWarning(f"Invalid repository type: '{repo_type}'")

# Check for org if modules repo
Expand Down Expand Up @@ -138,7 +138,7 @@ def get_components_to_install(subworkflow_dir: str) -> Tuple[List[str], List[str
"""
modules = []
subworkflows = []
with open(Path(subworkflow_dir, "main.nf"), "r") as fh:
with open(Path(subworkflow_dir, "main.nf")) as fh:
for line in fh:
regex = re.compile(
r"include(?: *{ *)([a-zA-Z\_0-9]*)(?: *as *)?(?:[a-zA-Z\_0-9]*)?(?: *})(?: *from *)(?:'|\")(.*)(?:'|\")"
Expand Down
Loading
Loading