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

Use ruff to lint Python code #518

Merged
merged 2 commits into from
Nov 9, 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
8 changes: 0 additions & 8 deletions .flake8

This file was deleted.

9 changes: 3 additions & 6 deletions .github/workflows/python-package.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ jobs:
test:
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
python-version: [ 3.8, 3.9, '3.10', '3.11', 3.12 ]
python-version: [ 3.8, 3.9, '3.10', 3.11, 3.12 ]

steps:
- uses: actions/checkout@v4
Expand Down Expand Up @@ -57,11 +58,7 @@ jobs:
python -m pip install tox-gh-actions
if [ -f requirements.txt ]; then pip install -r requirements.txt; fi
- name: Lint
run: |
# stop the build if there are Python syntax errors or undefined names
flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics
# exit-zero treats all errors as warnings. The GitHub editor is 127 chars wide
flake8 . --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics
run: ruff check --output-format=github .

typecheck:
runs-on: ubuntu-latest
Expand Down
11 changes: 6 additions & 5 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
repos:
- repo: https://github.com/pycqa/flake8
rev: '7.1.1'
hooks:
- id: flake8
exclude: docs/conf.py|test_settings.py
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: 'v5.0.0'
hooks:
- id: end-of-file-fixer
- id: trailing-whitespace

- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.7.3
hooks:
- id: ruff
4 changes: 2 additions & 2 deletions CONTRIBUTING.rst
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ To be mergeable, patches must:
- not change existing tests without a *very* good reason,
- add tests for new code (bug fixes should include regression tests, new
features should have relevant tests),
- not introduce any new flake8_ errors (run ``./run.sh lint``),
- not introduce any new ruff_ errors (run ``./run.sh lint``),
- not introduce any new mypy_ errors (run ``./run.sh typecheck``),
- include updated source translations (run ``./run.sh makemessages`` and ``./run.sh compilemessages``),
- document any new features, and
Expand All @@ -80,6 +80,6 @@ with it.

.. _open a new issue: https://github.com/jazzband/django-waffle/issues/new
.. _Fork: https://github.com/jazzband/django-waffle/fork
.. _flake8: https://pypi.python.org/pypi/flake8
.. _ruff: https://pypi.python.org/pypi/ruff
.. _mypy: https://www.mypy-lang.org/
.. _good commit message: http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html
4 changes: 2 additions & 2 deletions docs/about/contributing.rst
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ To be mergeable, patches must:
- not change existing tests without a *very* good reason,
- add tests for new code (bug fixes should include regression tests, new
features should have relevant tests),
- not introduce any new flake8_ errors (run ``./run.sh lint``),
- not introduce any new ruff_ errors (run ``./run.sh lint``),
- document any new features, and
- have a `good commit message`_.

Expand All @@ -70,5 +70,5 @@ with it.

.. _open a new issue: https://github.com/jazzband/django-waffle/issues/new
.. _Fork: https://github.com/jazzband/django-waffle/fork
.. _flake8: https://pypi.python.org/pypi/flake8
.. _ruff: https://pypi.python.org/pypi/ruff
.. _good commit message: http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html
3 changes: 2 additions & 1 deletion docs/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
# All configuration values have a default; values that are commented out
# serve to show the default.

import sys, os
import sys
import os

# If extensions (or modules to document with autodoc) are in another directory,
# add these directories to sys.path here. If the directory is relative to the
Expand Down
2 changes: 1 addition & 1 deletion docs/starting/configuring.rst
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ behavior.
The value describes the level of wanted warning, possible values are all levels know by pythons default logging,
e.g. ``logging.WARNING``.
Defaults to ``None``.


``WAFFLE_ENABLE_ADMIN_PAGES``
Enables the default admin pages for Waffle models. This is True by default,
Expand Down
2 changes: 1 addition & 1 deletion docs/usage/decorators.rst
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ Flags
@waffle_flag('flag_name')
def myview(request):
pass

@waffle_flag('flag_name', 'url_name_to_redirect_to')
def myotherview(request):
pass
Expand Down
4 changes: 2 additions & 2 deletions docs/usage/templates.rst
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ features on the front-end. It includes support for both Django's
built-in templates and for Jinja2_.

.. warning::

Before using samples in templates, see the warning in the
:ref:`Sample chapter <types-sample>`.

Expand Down Expand Up @@ -92,7 +92,7 @@ Switches
--------

::

{% if waffle.switch('switch_name') %}
switch_name is active!
{% endif %}
Expand Down
2 changes: 1 addition & 1 deletion docs/usage/views.rst
Original file line number Diff line number Diff line change
Expand Up @@ -49,5 +49,5 @@ Samples
Returns ``True`` if the sample is active, else ``False``.

.. warning::

See the warning in the :ref:`Sample chapter <types-sample>`.
87 changes: 87 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,90 @@ strict_equality = true
[[tool.mypy.overrides]]
module = ["django.*"]
ignore_missing_imports = true

[tool.ruff]
line-length = 120
target-version = "py38"

[tool.ruff.lint]
Copy link
Contributor

Choose a reason for hiding this comment

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

question: why this set of lint rules?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are the ones that currently pass. Future PRs can uncomment rules that might be of interest. We do not want this PR to contain so many changes that it slows the review process.

select = [
"AIR", # Airflow
"ASYNC", # flake8-async
"BLE", # flake8-blind-except
"C90", # McCabe cyclomatic complexity
"DJ", # flake8-django
"DTZ", # flake8-datetimez
"E", # pycodestyle errors
"F", # Pyflakes
"FIX", # flake8-fixme
"FLY", # flynt
"G", # flake8-logging-format
"ICN", # flake8-import-conventions
"INP", # flake8-no-pep420
"INT", # flake8-gettext
"NPY", # NumPy-specific rules
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: this library doesn't use numpy or pandas and seems unlikely to start doing so so these could be omitted for a minor gain in readability

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this library doesn't use numpy or pandas and seems unlikely to start doing so

Famous last words. You never know when code will be forked or when contributors will have new ideas. Given that ruff can lint the entire CPython codebase in less than a third of a second with no cache, I take the stance that if a family of rules pass then run them.

"PD", # pandas-vet
"PIE", # flake8-pie
"PL", # Pylint
"PYI", # flake8-pyi
"RSE", # flake8-raise
"SLOT", # flake8-slots
"T10", # flake8-debugger
"T20", # flake8-print
"TD", # flake8-todos
"TID", # flake8-tidy-imports
"UP", # pyupgrade
"W", # pycodestyle warnings
"YTT", # flake8-2020
# "A", # flake8-builtins
# "ANN", # flake8-annotations
# "ARG", # flake8-unused-arguments
# "B", # flake8-bugbear
# "C4", # flake8-comprehensions
# "COM", # flake8-commas
# "CPY", # flake8-copyright
# "D", # pydocstyle
# "EM", # flake8-errmsg
# "ERA", # eradicate
# "EXE", # flake8-executable
# "FA", # flake8-future-annotations
# "FBT", # flake8-boolean-trap
# "I", # isort
# "ISC", # flake8-implicit-str-concat
# "N", # pep8-naming
# "PERF", # Perflint
# "PGH", # pygrep-hooks
# "PT", # flake8-pytest-style
# "PTH", # flake8-use-pathlib
# "Q", # flake8-quotes
# "RET", # flake8-return
# "RUF", # Ruff-specific rules
# "S", # flake8-bandit
# "SIM", # flake8-simplify
# "SLF", # flake8-self
# "TCH", # flake8-type-checking
# "TRY", # tryceratops
]
# Files not checked:
# - migrations: most of these are autogenerated and don't need a check
# - docs: contains autogenerated code that doesn't need a check
exclude = [
"*/migrations/*",
"docs",
]
ignore = ["F401"]

[tool.ruff.lint.mccabe]
Copy link
Contributor

Choose a reason for hiding this comment

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

question: why complexity 23 ?

Copy link
Contributor Author

@cclauss cclauss Nov 9, 2024

Choose a reason for hiding this comment

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

Set it to 22 and see what happens...

% pipx run ruff check --config mccabe.max-complexity=22

max-complexity = 23

[tool.ruff.lint.per-file-ignores]
"docs/conf.py" = ["INP001"]
"test_app/models.py" = ["DJ008"] # FIXME
Copy link
Contributor

Choose a reason for hiding this comment

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

praise: nice to highlight this in the comment

"waffle/models.py" = ["DJ012", "PYI019"] # FIXME

[tool.ruff.lint.pylint]
allow-magic-value-types = ["float", "int", "str"]
max-args = 6 # default is 5
Copy link
Contributor

Choose a reason for hiding this comment

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

praise: nice to have the defaults mentioned here as a possible future target for refactoring

max-branches = 23 # default is 12
max-returns = 13 # default is 6
max-statements = 51 # default is 50
2 changes: 1 addition & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@ Django
django-jinja>=2.4.1,<3
transifex-client

flake8
mypy
ruff
tox
2 changes: 1 addition & 1 deletion requirements/test.txt
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
flake8
ruff
tox
4 changes: 2 additions & 2 deletions run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ export DJANGO_SETTINGS_MODULE="test_settings"
usage() {
echo "USAGE: $0 [command]"
echo " test - run the waffle tests"
echo " lint - run flake8"
echo " lint - run ruff"
echo " typecheck - run mypy"
echo " shell - open the Django shell"
echo " makemigrations - create a schema migration"
Expand All @@ -20,7 +20,7 @@ case "$CMD" in
"test" )
DJANGO_SETTINGS_MODULE=test_settings django-admin test waffle $@ ;;
"lint" )
flake8 waffle $@ ;;
ruff check ;;
"typecheck" )
mypy waffle $@ ;;
"shell" )
Expand Down
2 changes: 1 addition & 1 deletion test_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

# Make filepaths relative to settings.
ROOT = os.path.dirname(os.path.abspath(__file__))
path = lambda *a: os.path.join(ROOT, *a)
path = lambda *a: os.path.join(ROOT, *a) # noqa: E731

DEBUG = True
TEST_RUNNER = 'django.test.runner.DiscoverRunner'
Expand Down
10 changes: 3 additions & 7 deletions waffle/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def get_waffle_sample_model() -> type[AbstractBaseSample]:


def get_waffle_model(setting_name: str) -> (
type[AbstractBaseFlag] | type[AbstractBaseSwitch] | type[AbstractBaseSample]
type[AbstractBaseFlag | AbstractBaseSwitch | AbstractBaseSample]
):
"""
Returns the waffle Flag model that is active in this project.
Expand All @@ -63,12 +63,8 @@ def get_waffle_model(setting_name: str) -> (
try:
return django_apps.get_model(flag_model_name)
except ValueError:
raise ImproperlyConfigured("WAFFLE_{} must be of the form 'app_label.model_name'".format(
setting_name
))
raise ImproperlyConfigured(f"WAFFLE_{setting_name} must be of the form 'app_label.model_name'")
except LookupError:
raise ImproperlyConfigured(
"WAFFLE_{} refers to model '{}' that has not been installed".format(
setting_name, flag_model_name
)
f"WAFFLE_{setting_name} refers to model '{flag_model_name}' that has not been installed"
)
4 changes: 2 additions & 2 deletions waffle/locale/ru/LC_MESSAGES/django.po
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
# Copyright (C) YEAR THE PACKAGE'S COPYRIGHT HOLDER
# This file is distributed under the same license as the PACKAGE package.
# FIRST AUTHOR <EMAIL@ADDRESS>, YEAR.
#
#
# Translators:
# Clinton Blackburn <[email protected]>, 2021
#
#
#, fuzzy
msgid ""
msgstr ""
Expand Down
6 changes: 3 additions & 3 deletions waffle/management/commands/waffle_delete.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def handle(self, *args: Any, **options: Any) -> None:
flag_queryset = get_waffle_flag_model().objects.filter(name__in=flags)
flag_count = flag_queryset.count()
flag_queryset.delete()
self.stdout.write('Deleted %s Flags' % flag_count)
self.stdout.write(f'Deleted {flag_count} Flags')

switches = options['switch_names']
if switches:
Expand All @@ -47,11 +47,11 @@ def handle(self, *args: Any, **options: Any) -> None:
)
switch_count = switches_queryset.count()
switches_queryset.delete()
self.stdout.write('Deleted %s Switches' % switch_count)
self.stdout.write(f'Deleted {switch_count} Switches')

samples = options['sample_names']
if samples:
sample_queryset = get_waffle_sample_model().objects.filter(name__in=samples)
sample_count = sample_queryset.count()
sample_queryset.delete()
self.stdout.write('Deleted %s Samples' % sample_count)
self.stdout.write(f'Deleted {sample_count} Samples')
32 changes: 16 additions & 16 deletions waffle/management/commands/waffle_flag.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,19 +104,19 @@ def handle(self, *args: Any, **options: Any) -> None:
if options['list_flags']:
self.stdout.write('Flags:')
for flag in get_waffle_flag_model().objects.iterator():
self.stdout.write('NAME: %s' % flag.name)
self.stdout.write('SUPERUSERS: %s' % flag.superusers)
self.stdout.write('EVERYONE: %s' % flag.everyone)
self.stdout.write('AUTHENTICATED: %s' % flag.authenticated)
self.stdout.write('PERCENT: %s' % flag.percent)
self.stdout.write('TESTING: %s' % flag.testing)
self.stdout.write('ROLLOUT: %s' % flag.rollout)
self.stdout.write('STAFF: %s' % flag.staff)
self.stdout.write('GROUPS: %s' % list(
flag.groups.values_list('name', flat=True))
self.stdout.write(f'NAME: {flag.name}')
self.stdout.write(f'SUPERUSERS: {flag.superusers}')
self.stdout.write(f'EVERYONE: {flag.everyone}')
self.stdout.write(f'AUTHENTICATED: {flag.authenticated}')
self.stdout.write(f'PERCENT: {flag.percent}')
self.stdout.write(f'TESTING: {flag.testing}')
self.stdout.write(f'ROLLOUT: {flag.rollout}')
self.stdout.write(f'STAFF: {flag.staff}')
self.stdout.write('GROUPS: {}'.format(list(
flag.groups.values_list('name', flat=True)))
)
self.stdout.write('USERS: %s' % list(
flag.users.values_list(UserModel.USERNAME_FIELD, flat=True))
self.stdout.write('USERS: {}'.format(list(
flag.users.values_list(UserModel.USERNAME_FIELD, flat=True)))
)
self.stdout.write('')
return
Expand All @@ -129,7 +129,7 @@ def handle(self, *args: Any, **options: Any) -> None:
if options['create']:
flag, created = get_waffle_flag_model().objects.get_or_create(name=flag_name)
if created:
self.stdout.write('Creating flag: %s' % flag_name)
self.stdout.write(f'Creating flag: {flag_name}')
else:
try:
flag = get_waffle_flag_model().objects.get(name=flag_name)
Expand All @@ -149,7 +149,7 @@ def handle(self, *args: Any, **options: Any) -> None:
group_instance = Group.objects.get(name=group)
group_hash[group_instance.name] = group_instance.id
except Group.DoesNotExist:
raise CommandError('Group %s does not exist' % group)
raise CommandError(f'Group {group} does not exist')
# If 'append' was not passed, we clear related groups
if not options['append']:
flag.groups.clear()
Expand All @@ -168,11 +168,11 @@ def handle(self, *args: Any, **options: Any) -> None:
)
user_hash.add(user_instance)
except UserModel.DoesNotExist:
raise CommandError('User %s does not exist' % username)
raise CommandError(f'User {username} does not exist')
# If 'append' was not passed, we clear related users
if not options['append']:
flag.users.clear()
self.stdout.write('Setting user(s): %s' % user_hash)
self.stdout.write(f'Setting user(s): {user_hash}')
# for user in user_hash:
flag.users.add(*[user.id for user in user_hash])
elif hasattr(flag, option):
Expand Down
Loading
Loading