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 as default linting + formatting option #387

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 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
23 changes: 23 additions & 0 deletions ccds-help.json
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,29 @@
}
]
},
{
"field": "linting_and_formatting",
"help": {
"description": "How to handle linting and formatting on your code.",
"more_information": ""
},
"choices": [
{
"choice": "ruff",
"help": {
"description": "Use ruff for linting and formatting.",
"more_information": ""
}
},
{
"choice": "flake8+black+isort",
"help": {
"description": "Use flake8 for linting and black+isort for formatting.",
"more_information": ""
}
}
]
},
{
"field": "open_source_license",
"help": {
Expand Down
4 changes: 4 additions & 0 deletions ccds.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@
"none",
"basic"
],
"linting_and_formatting": [
"ruff",
"flake8+black+isort"
],
"open_source_license": ["No license file", "MIT", "BSD-3-Clause"],
"docs": ["mkdocs", "none"],
"include_code_scaffold": ["Yes", "No"]
Expand Down
9 changes: 7 additions & 2 deletions ccds/hook_utils/dependencies.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
packages = [
"pip",
"python-dotenv",
]

flake8_black_isort = [
"black",
"flake8",
"isort",
"pip",
"python-dotenv",
]

ruff = ["ruff"]

basic = [
"ipython",
"jupyterlab",
Expand Down
16 changes: 15 additions & 1 deletion hooks/post_gen_project.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,14 @@
# https://github.com/cookiecutter/cookiecutter/issues/824
# our workaround is to include these utility functions in the CCDS package
from ccds.hook_utils.custom_config import write_custom_config
from ccds.hook_utils.dependencies import basic, packages, scaffold, write_dependencies
from ccds.hook_utils.dependencies import (
basic,
flake8_black_isort,
packages,
ruff,
scaffold,
write_dependencies,
)

#
# TEMPLATIZED VARIABLES FILLED IN BY COOKIECUTTER
Expand All @@ -24,6 +31,13 @@
packages_to_install += basic
# {% endif %}

# {% if cookiecutter.linting_and_formatting == "ruff" %}
packages_to_install += ruff
# Remove setup.cfg
Path("setup.cfg").unlink()
# {% elif cookiecutter.linting_and_formatting == "flake8+black+isort" %}
packages_to_install += flake8_black_isort
# {% endif %}
# track packages that are not available through conda
pip_only_packages = [
"awscli",
Expand Down
2 changes: 2 additions & 0 deletions tests/conda_harness.sh
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,7 @@ make
make create_environment
conda activate $PROJECT_NAME
make requirements
make lint
make format

run_tests $PROJECT_NAME $MODULE_NAME
1 change: 1 addition & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ def _is_valid(config):
"dataset_storage",
"open_source_license",
"include_code_scaffold",
"linting_and_formatting",
"docs",
]
cyclers = {k: cycle(cookiecutter_json[k]) for k in cycle_fields}
Expand Down
4 changes: 4 additions & 0 deletions tests/pipenv_harness.sh
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ make create_environment
# can happen outside of environment since pipenv knows based on Pipfile
make requirements

# linting + formatting must happen inside environment
pipenv run make lint
pipenv run make format

# test with pipenv run
pipenv run python -c "import sys; assert \"$PROJECT_NAME\" in sys.executable"

Expand Down
28 changes: 13 additions & 15 deletions tests/test_creation.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ def test_baking_configs(config, fast):
with bake_project(config) as project_directory:
verify_folders(project_directory, config)
verify_files(project_directory, config)
lint(project_directory)

if fast < 2:
verify_makefile_commands(project_directory, config)
Expand Down Expand Up @@ -100,7 +99,6 @@ def verify_files(root, config):
"Makefile",
"README.md",
"pyproject.toml",
"setup.cfg",
".env",
".gitignore",
"data/external/.gitkeep",
Expand All @@ -120,6 +118,9 @@ def verify_files(root, config):
if not config["open_source_license"].startswith("No license"):
expected_files.append("LICENSE")

if config["linting_and_formatting"] == "flake8+black+isort":
expected_files.append("setup.cfg")

if config["include_code_scaffold"] == "Yes":
expected_files += [
f"{config['module_name']}/config.py",
Expand Down Expand Up @@ -156,6 +157,8 @@ def verify_makefile_commands(root, config):
- blank command listing commands
- create_environment
- requirements
- linting
- formatting
Ensure that these use the proper environment.
"""
test_path = Path(__file__).parent
Expand Down Expand Up @@ -184,23 +187,18 @@ def verify_makefile_commands(root, config):
stdout=PIPE,
)

stdout_output, _ = _decode_print_stdout_stderr(result)
stdout_output, stderr_output = _decode_print_stdout_stderr(result)

# Check that makefile help ran successfully
assert "Available rules:" in stdout_output
assert "clean Delete all compiled Python files" in stdout_output

assert result.returncode == 0


def lint(root):
"""Run the linters on the project."""
result = run(
["make", "lint"],
cwd=root,
stderr=PIPE,
stdout=PIPE,
)
_, _ = _decode_print_stdout_stderr(result)
# Check that linting and formatting ran successfully
if config["linting_and_formatting"] == "ruff":
assert "All checks passed!" in stdout_output
assert "reformatted" not in stdout_output
elif config["linting_and_formatting"] == "flake8+black+isort":
assert "All done!" in stderr_output
assert "reformatted" not in stderr_output

assert result.returncode == 0
2 changes: 2 additions & 0 deletions tests/virtualenv_harness.sh
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ else
fi

make requirements
make lint
make format

run_tests $PROJECT_NAME $MODULE_NAME

22 changes: 18 additions & 4 deletions {{ cookiecutter.repo_name }}/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ PYTHON_INTERPRETER = python
#################################################################################

{% if cookiecutter.dependency_file != 'none' %}
## Install Python Dependencies
## Install Python dependencies
.PHONY: requirements
requirements:
{% if "requirements.txt" == cookiecutter.dependency_file -%}
Expand All @@ -31,7 +31,19 @@ clean:
find . -type f -name "*.py[co]" -delete
find . -type d -name "__pycache__" -delete

## Lint using flake8 and black (use `make format` to do formatting)
{% if cookiecutter.linting_and_formatting == 'ruff' %}
## Lint using ruff (use `make format` to do formatting)
.PHONY: lint
lint:
ruff check {{ cookiecutter.module_name }}

## Format source code with ruff
.PHONY: format
format:
ruff check --select I --fix {{ cookiecutter.module_name }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like --select I here. We should use the fixable or unfixable configuration in pyproject.toml.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed on removing --select. Do you have an opinion on whether we should keep the default (everything is fixable) or specify fixable vs unfixable categories?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my other projects so far, I've been setting F as unfixable and it's been working for me so far. I haven't looked that hard at the rulesets though.

https://github.com/drivendataorg/erdantic/blob/664a3f71e22225635bc95e26578312c9b19dd363/pyproject.toml#L54-L60

ruff format {{ cookiecutter.module_name }}
{% elif cookiecutter.linting_and_formatting == 'flake8+black+isort' %}
## Lint using flake8, black, and isort (use `make format` to do formatting)
.PHONY: lint
lint:
flake8 {{ cookiecutter.module_name }}
Expand All @@ -41,7 +53,9 @@ lint:
## Format source code with black
.PHONY: format
format:
isort --profile black {{ cookiecutter.module_name }}
chrisjkuch marked this conversation as resolved.
Show resolved Hide resolved
black --config pyproject.toml {{ cookiecutter.module_name }}
{% endif %}

{% if not cookiecutter.dataset_storage.none %}
## Download Data from storage system
Expand Down Expand Up @@ -72,7 +86,7 @@ sync_data_up:
{% endif %}

{% if cookiecutter.environment_manager != 'none' %}
## Set up python interpreter environment
## Set up Python interpreter environment
.PHONY: create_environment
create_environment:
{% if cookiecutter.environment_manager == 'conda' -%}
Expand All @@ -97,7 +111,7 @@ create_environment:
#################################################################################

{% if cookiecutter.include_code_scaffold == 'Yes' %}
## Make Dataset
## Make dataset
.PHONY: data
data: requirements
$(PYTHON_INTERPRETER) {{ cookiecutter.module_name }}/data/make_dataset.py
Expand Down
14 changes: 10 additions & 4 deletions {{ cookiecutter.repo_name }}/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ classifiers = [
]
requires-python = "~={{ cookiecutter.python_version_number }}"

{% if cookiecutter.linting_and_formatting == 'flake8+black+isort' %}
[tool.black]
line-length = 99
include = '\.pyi?$'
Expand All @@ -25,8 +26,13 @@ exclude = '''
\.git
| \.venv
)/
'''

[tool.ruff.lint.isort]
known_first_party = ["{{ cookiecutter.module_name }}"]
force_sort_within_sections = true
[tool.isort]
profile = "black"
'''
{% endif %}
{% if cookiecutter.linting_and_formatting == 'ruff' %}
[tool.ruff]
line-length = 99
src = ["{{ cookiecutter.module_name }}"]
{% endif %}
Loading