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

Introduce ruff #753

Merged
merged 10 commits into from
Dec 12, 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
10 changes: 4 additions & 6 deletions {{ cookiecutter.name }}/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,16 @@ SIMULTANEOS_TEST_JOBS = 4
manage = poetry run python src/manage.py

fmt:
poetry run autoflake --in-place --remove-all-unused-imports --recursive src
poetry run isort src
poetry run black src
poetry run ruff check src --fix --unsafe-fixes
poetry run ruff format src
poetry run toml-sort pyproject.toml

lint:
$(manage) check
$(manage) makemigrations --check --dry-run --no-input

poetry run isort --check-only src
poetry run black --check src
poetry run flake8 src
poetry run ruff check src
poetry run ruff format --check src
poetry run mypy src
poetry run toml-sort pyproject.toml --check
poetry run pymarkdown scan README.md
Expand Down
2 changes: 1 addition & 1 deletion {{ cookiecutter.name }}/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ make test # run tests
### Style

* Obey [django's style guide](https://docs.djangoproject.com/en/dev/internals/contributing/writing-code/coding-style/#model-style).
* Configure your IDE to use [flake8](https://pypi.python.org/pypi/flake8) for checking your python code. To run our linters manualy, do `make lint`.
* Configure your IDE to use [ruff](https://pypi.org/project/ruff) for checking your Python code. To run our linters manually, do `make lint`. Feel free to [adjust](https://docs.astral.sh/ruff/configuration/) ruff [rules](https://docs.astral.sh/ruff/rules/) in `pyproject.toml` section `tool.ruff.lint` for your needs.
Copy link
Contributor Author

@da-maltsev da-maltsev Dec 11, 2024

Choose a reason for hiding this comment

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

Добавил немного инфы и ссылок для ruff. Немного отдает rtfm, но для линтера это подходящее настроение 😄

* Prefer English over your native language in comments and commit messages.
* Commit messages should contain the unique id of issue they are linked to (refs #100500).
* Every model, service and model method should have a docstring.
Expand Down
164 changes: 116 additions & 48 deletions {{ cookiecutter.name }}/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
build-backend = "poetry.core.masonry.api"
requires = ["poetry-core"]

[project]
requires-python = "~=3.11"
Copy link
Contributor

Choose a reason for hiding this comment

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

Интересно, а у нас нет в шаблоне других хардкодов версии питона? Чтобы их тоже настроить правильно. Может где-то в генерируемом для сервиса CI\CD?

Если нет и это единственное место, то я бы добавил пару уточнений в ридми про то как работать с игнором ruff в нашем проекте и можно мержить

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Вообще хардкод версии есть в docker-compose и тут же в pyproject для poetry.
Poetry по-другому не умеет и на requires-python не смотрит :(
А насчет docker compose особо и не знаю, что тут можно сделать, обычно же из env как вариант брать, но тут это не очень в тему.

Copy link
Contributor

Choose a reason for hiding this comment

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

Походу пора менять poetry на uv https://github.com/astral-sh/uv


[tool.poetry]
authors = ["Fedor Borshev <[email protected]>"]
description = ""
Expand Down Expand Up @@ -34,27 +37,11 @@ sentry-sdk = "^2.11.0"
whitenoise = "^6.7.0"

[tool.poetry.group.dev.dependencies]
autoflake = "^2.2.1"
black = "^24.4.2"
django-stubs = "^5.1.1"
djangorestframework-stubs = "^3.15.0"
dotenv-linter = "^0.5.0"
flake8-absolute-import = "^1.0.0.2"
flake8-bugbear = "^24.4.26"
flake8-cognitive-complexity = "^0.1.0"
flake8-django = "^1.4"
flake8-eradicate = "^1.5.0"
flake8-fixme = "^1.1.1"
flake8-pep3101 = "^2.1.0"
flake8-pie = "^0.16.0"
flake8-print = "^5.0.0"
flake8-printf-formatting = "^1.1.2"
flake8-pyproject = "^1.2.3"
flake8-variables-names = "^0.0.6"
flake8-walrus = "^1.2.0"
freezegun = "^1.5.1"
ipython = "^8.26.0"
isort = "^5.12.0"
jedi = "^0.19.1"
mixer = {extras = ["django"], version = "^7.2.2"}
mypy = "^1.11.0"
Expand All @@ -66,42 +53,11 @@ pytest-freezer = "^0.4.8"
pytest-mock = "^3.12.0"
pytest-randomly = "^3.15.0"
pytest-xdist = "^3.6.1"
ruff = "^0.8.0"
toml-sort = "^0.23.1"
types-freezegun = "^1.1.10"
types-pillow = "^10.2.0.20240520"

[tool.black]
exclude = '''
/(
| migrations
)/
'''
line_length = 160

[tool.flake8]
exclude = ["__pycache__", "migrations"]
ignore = [
"E203", # whitespace before ':'
"E265", # block comment should start with '#'
"E501", # line too long ({} > {} characters)
"E704", # multiple statements on one line (def)
"F811", # redefinition of unused name from line {}
"PT001", # use @pytest.fixture() over @pytest.fixture
"SIM102", # use a single if-statement instead of nested if-statements
"SIM113", # use enumerate instead of manually incrementing a counter
]
inline-quotes = '"'

[tool.isort]
include_trailing_comma = true
line_length = 160
multi_line_output = 3
skip = [
"migrations",
]
src_paths = ["src", "tests"]
use_parentheses = true

[tool.pymarkdown.plugins.md013]
enabled = false

Expand All @@ -123,6 +79,118 @@ filterwarnings = [
python_files = ["test*.py"]
pythonpath = ". src"

[tool.ruff]
exclude = ["__pycache__", "migrations"]
line-length = 160
src = ["src"]

[tool.ruff.lint]
ignore = [
Copy link
Contributor

Choose a reason for hiding this comment

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

Хм, а можно TLDR?

Типа эти штуки у нас не использовались? И почему мы не хотим их использовать?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Вот сводка по каждой группе в игноре:

  • A... - правила из flake8-builtins, раньше не использовали, в целом, можно и включить
  • ANN401 - не использовалось, запрещает typing.Any (ну это перебор на мой взгляд)
  • ARG002, ARG005 - не использовалось, и может ломать интрефейсы/полиморфизм
  • B019 - запрещает functools.lru_cache/cache в классах, кажется избыточным
  • B904 - заставляет кидать исключения строго с from, кажется избыточным
  • C408 - не использовалось, заставляет использовать [](){} вместо list, tuple, dict . На мой взгляд - это не всегда нужно и текстовые варианты более читаемы бывают.
  • D... - куча обязаловки для написания docstrings для всех публичных сущностей, может быть нужно для библиотек, но для бизнес кода это оверкил
  • DTZ001 - запрещает datetime без таймзоны, кажется избыточным ограничением. Там где нужна таймзона пользуемся джанговским хелпером
  • E501 - и так был в игноре, ограничение на макс длину линии
  • EM101, EM102 - не нужно, т.к. запрщает писать сообщение ошибки сразу в Excepton, a требует вынести его в отдельную переменную
  • FBT - запрещает bool флаги в функциях, в общем то кажется полезным, но не уверен стоит ли включать
  • INP - обязаловка иметь init.py , избыточно да и вредно местами
  • INT001 - бан f-строк в gettext штуках, кажется не нужным
  • N... - куча правил по неймингу аргументов, методов, модулей и прочего, в игноре т.к. конфликтуют с частыми джанговскими патернами типа импорта моделей из apps и подобного
  • PERF401 - принуждает к comprehensions вместо for-loop, но иногда это может вредить читаемости, так что игнор
  • PGH - обязаловка указывать игноры для линтера и тайпчекера конкретно типа type: ignore -> type:ignore[override], кажется избыточным
  • PLR0913 - ограничевает количество аргументов в функциях, дефолтно макс 5 аргументов, может быть
  • PT001 - раньше тоже игнор был, чтобы использовать pytest.fixture а не pytest.fixture()
  • RET501-503 - избыточно, т.к. местами заставляет писать лишние return None, где это и так происходит автоматом.
  • RUF015 - избыточно, next(iter(x)) вместо list(x)[0]
  • S101 - банит использование assert, кажется не нужным ограничением
  • S311 - принуждает использовать безопасный random - избыточно и может мешать
  • S324 - банит ряд хешфункций, излишне
  • SIM102 - ранее тоже было в игноре
  • SIM113 - ранее было в игноре
  • TC001-003 - принуждает прятать импорты в TYPE_CHECKING всегда, когда возможно - изыбточно, да и ломает lsp в некоторых редакторах
  • TRY003 TRY300 - переусложнение работы с исключениями, может еще и навредить
  • RUF012 - не очень дружит с django, постоянно требует лишних аннотаций

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Кажется, что делать сноску в README по каждому правилу, как я выше расписал, получится капец как жирно.
В целом, думал, что коммент с основной инфой об исключенном правиле, как это уже сделано, должен уже давать достаточно понимания.
Мб считаешь иначе, помоги тогда придумать формат, как и где лучше дать этот tldr

Copy link
Contributor

Choose a reason for hiding this comment

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

Спасибо! Когда я говорил про ридми, я имел ввиду указать, что у нас используется ruff и указать, как работать с ним и правильно настраивать самим игнор лист. А то есть ребята, которые этого могут не понять и охуеть от этих ограничений и пошлют нахуй наш темплейт и не будут ставить ему звездочки

"A001", # variable `{}` is shadowing a Python builtin
"A002", # argument `{}` is shadowing a Python builtin
"A003", # class attribute `{}` is shadowing a Python builtin
"ANN401", # dynamically typed expressions (typing.Any) are disallowed in `{}`
"ARG002", # unused method argument: `{}`
"ARG005", # unused lambda argument: `{}`
"B018", # found useless expression. Either assign it to a variable or remove it
"B904", # within an `except` clause, raise exceptions with [...]
"C408", # unnecessary `dict` call (rewrite as a literal)
"D100", # missing docstring in public module
"D101", # missing docstring in public class
"D102", # missing docstring in public method
"D103", # missing docstring in public function
"D104", # missing docstring in public package
"D105", # missing docstring in magic method
"D106", # missing docstring in public nested class
"D107", # missing docstring in `__init__`
"D200", # one-line docstring should fit on one line
"D202", # no blank lines allowed after function docstring (found {})
"D203", # 1 blank line required before class docstring
"D205", # 1 blank line required between summary line and description
"D209", # multi-line docstring closing quotes should be on a separate line
"D210", # no whitespaces allowed surrounding docstring text
"D212", # multi-line docstring summary should start at the first line
"D213", # multi-line docstring summary should start at the second line
"D400", # first line should end with a period
"D401", # first line of docstring should be in imperative mood: "{}"
"D404", # first word of the docstring should not be "This"
"D415", # first line should end with a period, question mark, or exclamation point
"DTZ001", # the use of `datetime.datetime()` without `tzinfo` argument is not allowed
"E501", # line too long ({} > {})
"EM101", # exception must not use a string literal, assign to variable first
"EM102", # expection must not use an f-string literal, assign to variable first
"FBT001", # boolean-typed position argument in function definition
"FBT002", # boolean default position argument in function definition
"FBT003", # boolean positional value in function call
"INP001", # file `{}` is part of an implicit namespace package. Add an `__init__.py`
"INT001", # f-string is resolved before function call; consider `_("string %s") % arg`
"N802", # function name `{}` should be lowercase
"N803", # argument name `{}` should be lowercase
"N804", # first argument of a class method should be named `cls`
"N806", # variable `{}` in function should be lowercase
"N812", # lowercase `{}` imported as non-lowercase `{}`
"N818", # exception name `{}` should be named with an Error suffix
"N999", # invalid module name: '{}'
"PERF401", # use a list comprehension to create a transformed list
"PGH003", # use specific rule codes when ignoring type issues
"PGH004", # use specific rule codes when using `noqa`
"PT001", # use `@pytest.fixture()` over `@pytest.fixture`
"RET501", # do not explicitly `return None` in function if it is the only possible return value
"RET502", # do not implicitly `return None` in function able to return non-`None` value
"RET503", # missing explicit `return` at the end of function able to return non-`None` value
"RUF012", # mutable class attributes should be annotated with `typing.ClassVar`
"RUF015", # prefer next({iterable}) over single element slice
"S101", # use of `assert` detected
"S311", # standart pseudo-random generators are not suitable for cryptographic purposes
"S324", # probable use of insecure hash functions in `{}`: `{}`
"SIM102", # use a single `if` statement instead of nested `if` statements
"SIM108", # use ternary operator `{}` instead of `if`-`else`-block
"SIM113", # use enumerate instead of manually incrementing a counter
"TC001", # move application import `{}` into a type-checking block
"TC002", # move third-party import `{}` into a type-checking block
"TC003", # move standart library import `{}` into a type-checking block
"TRY003", # avoid specifying long messages outside the exception class
"TRY300", # consider moving this statement to an `else` block
]
select = ["ALL"]

[tool.ruff.lint.flake8-tidy-imports]
ban-relative-imports = "all"

[tool.ruff.lint.isort]
combine-as-imports = true
known-first-party = ["src"]
lines-after-imports = 2

[tool.ruff.lint.per-file-ignores]
"*/factory.py" = [
"ANN", # flake8-annotations
"ARG001",
]
"*/fixtures.py" = [
"ANN", # flake8-annotations
"ARG001",
]
"*/management/*" = [
"ANN", # flake8-annotations
]
"*/migrations/*" = [
"ANN", # flake8-annotations
]
"*/tests/*" = [
"ANN", # flake8-annotations
"ARG001",
"PLR2004",
]
"src/app/conf/*" = [
"ANN", # flake8-annotations
]
"src/app/testing/*" = [
"ANN", # flake8-annotations
"ARG001",
"PLR2004",
]

[tool.tomlsort]
all = true
in_place = true
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from app.admin import ModelAdmin, admin


__all__ = [
"admin",
"ModelAdmin",
"admin",
]
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from django.urls import include, path
from rest_framework.routers import SimpleRouter


router = SimpleRouter()

urlpatterns = [
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
from app.models import DefaultModel, TimestampedModel, models


__all__ = [
"models",
"DefaultModel",
"TimestampedModel",
"models",
]
1 change: 1 addition & 0 deletions {{ cookiecutter.name }}/src/a12n/api/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from a12n.api import views


app_name = "a12n"

urlpatterns = [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import pytest
from axes.models import AccessAttempt


pytestmark = pytest.mark.django_db


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

from a12n.utils import get_jwt


pytestmark = [
pytest.mark.django_db,
pytest.mark.freeze_time("2049-01-05"),
Expand Down
3 changes: 2 additions & 1 deletion {{ cookiecutter.name }}/src/app/admin/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@

from app.admin.model_admin import ModelAdmin


__all__ = [
"admin",
"ModelAdmin",
"admin",
]
9 changes: 5 additions & 4 deletions {{ cookiecutter.name }}/src/app/api/viewsets.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from typing import Any, Optional, Protocol, Type
from typing import Any, Protocol

from rest_framework import mixins, status
from rest_framework.mixins import CreateModelMixin, UpdateModelMixin
Expand All @@ -7,6 +7,7 @@
from rest_framework.serializers import BaseSerializer
from rest_framework.viewsets import GenericViewSet


__all__ = ["DefaultModelViewSet"]


Expand Down Expand Up @@ -53,7 +54,7 @@ def update(self: BaseGenericViewSet, request: Request, *args: Any, **kwargs: Any
if getattr(instance, "_prefetched_objects_cache", None):
# If 'prefetch_related' has been applied to a queryset, we need to
# forcibly invalidate the prefetch cache on the instance.
instance._prefetched_objects_cache = {}
instance._prefetched_objects_cache = {} # noqa: SLF001

return self.get_response(instance, status.HTTP_200_OK)

Expand Down Expand Up @@ -101,8 +102,8 @@ def get_response(

def get_serializer_class(
self: BaseGenericViewSet,
action: Optional[str] = None,
) -> Type[BaseSerializer]:
action: str | None = None,
) -> type[BaseSerializer]:
if action is None:
action = self.action # type: ignore

Expand Down
1 change: 1 addition & 0 deletions {{ cookiecutter.name }}/src/app/celery.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from celery import Celery
from django.conf import settings


__all__ = ["celery"]

os.environ.setdefault("DJANGO_SETTINGS_MODULE", "app.settings")
Expand Down
1 change: 1 addition & 0 deletions {{ cookiecutter.name }}/src/app/conf/api.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from app.conf.environ import env


# Django REST Framework
# https://www.django-rest-framework.org/api-guide/settings/

Expand Down
1 change: 1 addition & 0 deletions {{ cookiecutter.name }}/src/app/conf/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from app.conf.environ import env


AUTH_USER_MODEL = "users.User"
AXES_ENABLED = env("AXES_ENABLED", cast=bool, default=True)

Expand Down
1 change: 1 addition & 0 deletions {{ cookiecutter.name }}/src/app/conf/boilerplate.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from pathlib import Path


BASE_DIR = Path(__file__).resolve().parent.parent

ROOT_URLCONF = "app.urls"
Expand Down
1 change: 1 addition & 0 deletions {{ cookiecutter.name }}/src/app/conf/celery.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from app.conf.environ import env
from app.conf.timezone import TIME_ZONE


CELERY_BROKER_URL = env("CELERY_BROKER_URL", cast=str, default="redis://localhost:6379/0")
CELERY_TASK_ALWAYS_EAGER = env("CELERY_TASK_ALWAYS_EAGER", cast=bool, default=env("DEBUG"))
CELERY_TIMEZONE = TIME_ZONE
Expand Down
1 change: 1 addition & 0 deletions {{ cookiecutter.name }}/src/app/conf/db.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

from app.conf.environ import env


DATABASES = {
# read os.environ["DATABASE_URL"] and raises ImproperlyConfigured exception if not found
"default": env.db(),
Expand Down
1 change: 1 addition & 0 deletions {{ cookiecutter.name }}/src/app/conf/environ.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from app.conf.boilerplate import BASE_DIR


env = environ.Env(
DEBUG=(bool, False),
CI=(bool, False),
Expand Down
1 change: 1 addition & 0 deletions {{ cookiecutter.name }}/src/app/conf/http.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from app.conf.environ import env


ALLOWED_HOSTS = ["*"] # host validation is not necessary in 2020
CSRF_TRUSTED_ORIGINS = [
"http://your.app.origin",
Expand Down
1 change: 1 addition & 0 deletions {{ cookiecutter.name }}/src/app/conf/i18n.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from app.conf.boilerplate import BASE_DIR


LANGUAGE_CODE = "ru"

LOCALE_PATHS = [Path(BASE_DIR).parent / "locale"]
Expand Down
Loading
Loading