Skip to content

Commit

Permalink
style: replace black+isort+flake8 tooling with ruff (#1466)
Browse files Browse the repository at this point in the history
Code style and format changes moving from black+isort+flake8 to ruff.
  • Loading branch information
egparedes authored Feb 28, 2024
2 parents 563fe29 + acb1fef commit 6a19355
Show file tree
Hide file tree
Showing 158 changed files with 1,184 additions and 1,307 deletions.
128 changes: 14 additions & 114 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,10 @@
default_language_version:
python: python3.10
repos:
# - repo: meta
# hooks:
# - id: check-hooks-apply
# - id: check-useless-excludes

# - repo: meta
# hooks:
# - id: check-hooks-apply
# - id: check-useless-excludes
- repo: https://github.com/macisamuele/language-formatters-pre-commit-hooks
rev: v2.6.0
hooks:
Expand All @@ -33,20 +32,6 @@ repos:
types: [python]
args: [--comment-style, "|#|", --license-filepath, ./LICENSE_HEADER.txt, --fuzzy-match-generates-todo]

# - repo: https://github.com/asottile/yesqa
# rev: v1.4.0
# hooks:
# - id: yesqa
# additional_dependencies:
# - flake8==5.0.4 # version from constraints.txt
# - darglint
# - flake8-bugbear
# - flake8-builtins
# - flake8-debugger
# - flake8-docstrings
# - flake8-eradicate
# - flake8-mutable

- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.4.0
hooks:
Expand All @@ -56,106 +41,21 @@ repos:
- id: check-yaml
- id: debug-statements

- repo: https://github.com/psf/black
##[[[cog
## import re
## version = re.search('black==([0-9\.]*)', open("constraints.txt").read())[1]
## print(f"rev: '{version}' # version from constraints.txt")
##]]]
rev: '24.2.0' # version from constraints.txt
##[[[end]]]
hooks:
- id: black

# - repo: https://github.com/charliermarsh/ruff-pre-commit
# ##[[[cog
# ## import re
# ## version = re.search('ruff==([0-9\.]*)', open("constraints.txt").read())[1]
# ## print(f"# rev: 'v{version}' # version from constraints.txt")
# ##]]]
# rev: 'v0.2.2' # version from constraints.txt
# ##[[[end]]]
# hooks:
# - id: ruff
# # args: [ --fix, --exit-non-zero-on-fix ]

- repo: https://github.com/PyCQA/isort
##[[[cog
## import re
## version = re.search('isort==([0-9\.]*)', open("constraints.txt").read())[1]
## print(f"rev: '{version}' # version from constraints.txt")
##]]]
rev: '5.13.2' # version from constraints.txt
##[[[end]]]
hooks:
- id: isort

- repo: https://github.com/PyCQA/flake8
##[[[cog
## import re
## version = re.search('flake8==([0-9\.]*)', open("constraints.txt").read())[1]
## print(f"rev: '{version}' # version from constraints.txt")
##]]]
rev: '7.0.0' # version from constraints.txt
##[[[end]]]
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.2.0
hooks:
- id: flake8
additional_dependencies:
##[[[cog
## import re
## constraints = open("constraints.txt").read()
## for pkg in ["darglint", "flake8-bugbear", "flake8-builtins", "flake8-debugger", "flake8-docstrings",
## "flake8-eradicate", "flake8-mutable", "flake8-pyproject", "pygments"]:
## print(f"- {pkg}==" + str(re.search(f'\n{pkg}==([0-9\.]*)', constraints)[1]))
##]]]
- darglint==1.8.1
- flake8-bugbear==24.2.6
- flake8-builtins==2.2.0
- flake8-debugger==4.1.2
- flake8-docstrings==1.7.0
- flake8-eradicate==1.5.0
- flake8-mutable==1.2.0
- flake8-pyproject==1.2.3
- pygments==2.17.2
##[[[end]]]
# - flake8-rst-docstrings # Disabled for now due to random false positives
exclude: |
(?x)^(
setup.py |
docs/user/cartesian/conf.py |
src/gt4py/cartesian/__gtscript__.py |
src/gt4py/cartesian/__init__.py |
src/gt4py/cartesian/gtscript.py |
src/gt4py/cartesian/backend/__init__.py |
src/gt4py/cartesian/backend/pyext_builder.py |
src/gt4py/cartesian/frontend/__init__.py |
src/gt4py/cartesian/frontend/nodes.py |
src/gt4py/cartesian/frontend/node_util.py |
src/gt4py/cartesian/utils/__init__.py |
src/gt4py/cartesian/utils/base.py |
src/gt4py/cartesian/utils/attrib.py |
src/gt4py/cartesian/utils/meta.py |
src/gt4py/eve/extended_typing.py |
tests/conftest.py |
tests/cartesian_tests/integration_tests/multi_feature_tests/stencil_definitions.py |
tests/cartesian_tests/integration_tests/multi_feature_tests/test_code_generation.py |
tests/cartesian_tests/integration_tests/multi_feature_tests/utils.py |
tests/cartesian_tests/integration_tests/feature_tests/test_call_interface.py |
tests/cartesian_tests/unit_tests/frontend_tests/test_gtscript_frontend.py |
tests/next_tests/unit_tests/.* |
tests/next_tests/integration_tests/multi_feature_tests/.* |
tests/next_tests/integration_tests/feature_tests/ffront_tests/.* |
tests/next_tests/integration_tests/feature_tests/iterator_tests/.* |
tests/next_tests/integration_tests/feature_tests/otf_tests/.* |
tests/next_tests/integration_tests/feature_tests/math_builtin_test_data.py |
tests/next_tests/past_common_fixtures.py |
tests/next_tests/toy_connectivity.py |
)$
# Run the linter.
# TODO: include tests here
- id: ruff
files: ^src/
args: [--fix]
# Run the formatter.
- id: ruff-format

- repo: https://github.com/pre-commit/mirrors-mypy
##[[[cog
## import re
## version = re.search('mypy==([0-9\.]*)', open("constraints.txt").read())[1]
## version = re.search('mypy==([0-9\.]*)', open("constraints.txt").read())[1]
## print(f"#========= FROM constraints.txt: v{version} =========")
##]]]
#========= FROM constraints.txt: v1.8.0 =========
Expand Down
25 changes: 17 additions & 8 deletions CODING_GUIDELINES.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ We follow the [Google Python Style Guide][google-style-guide] with a few minor c

We deviate from the [Google Python Style Guide][google-style-guide] only in the following points:

- We use [`flake8`][flake8] with some plugins instead of [`pylint`][pylint].
- We use [`black`][black] and [`isort`][isort] for source code and imports formatting, which may work differently than indicated by the guidelines in section [_3. Python Style Rules_](https://google.github.io/styleguide/pyguide.html#3-python-style-rules). For example, maximum line length is set to 100 instead of 79 (although docstring lines should still be limited to 79).
- We use [`ruff-linter`][ruff-linter] instead of [`pylint`][pylint].
- We use [`ruff-formatter`][ruff-formatter] for source code and imports formatting, which may work differently than indicated by the guidelines in section [_3. Python Style Rules_](https://google.github.io/styleguide/pyguide.html#3-python-style-rules). For example, maximum line length is set to 100 instead of 79 (although docstring lines should still be limited to 79).
- According to subsection [_2.19 Power Features_](https://google.github.io/styleguide/pyguide.html#219-power-features), direct use of _power features_ (e.g. custom metaclasses, import hacks, reflection) should be avoided, but standard library classes that internally use these power features are accepted. Following the same spirit, we allow the use of power features in infrastructure code with similar functionality and scope as the Python standard library.
- According to subsection [_3.19.12 Imports For Typing_](https://google.github.io/styleguide/pyguide.html#31912-imports-for-typing), symbols from `typing` and `collections.abc` modules used in type annotations _"can be imported directly to keep common annotations concise and match standard typing practices"_. Following the same spirit, we allow symbols to be imported directly from third-party or internal modules when they only contain a collection of frequently used typying definitions.

Expand Down Expand Up @@ -107,7 +107,7 @@ In general, you should structure new Python modules in the following way:
1. _shebang_ line: `#! /usr/bin/env python3` (only for **executable scripts**!).
2. License header (see `LICENSE_HEADER.txt`).
3. Module docstring.
4. Imports, alphabetically ordered within each block (fixed automatically by `isort`):
4. Imports, alphabetically ordered within each block (fixed automatically by `ruff-formatter`):
1. Block of imports from the standard library.
2. Block of imports from general third party libraries using standard shortcuts when customary (e.g. `numpy as np`).
3. Block of imports from specific modules of the project.
Expand All @@ -126,10 +126,17 @@ Consider configuration files as another type of source code and apply the same c

### Ignoring QA errors

You may occasionally need to disable checks from _quality assurance_ (QA) tools (e.g. linters, type checkers, etc.) on specific lines as some tool might not be able to fully understand why a certain piece of code is needed. This is usually done with special comments, e.g. `# type: ignore`. However, you should **only** ignore QA errors when you fully understand their source and rewriting your code to pass QA checks would make it less readable. Additionally, you should add a brief comment for future reference, e.g.:
You may occasionally need to disable checks from _quality assurance_ (QA) tools (e.g. linters, type checkers, etc.) on specific lines as some tool might not be able to fully understand why a certain piece of code is needed. This is usually done with special comments, e.g. `# noqa: F401`, `# type: ignore`. However, you should **only** ignore QA errors when you fully understand their source and rewriting your code to pass QA checks would make it less readable. Additionally, you should add a short descriptive code if possible (check [ruff rules][ruff-rules] and [mypy error codes][mypy-error-codes] for reference):

```python
f = lambda: 'empty' # noqa: E731 # assign lambda expression for testing
f = lambda: 'empty' # noqa: E731 [lambda-assignment]
```

and, if needed, a brief comment for future reference:

```python
...
return undeclared_symbol # noqa: F821 [undefined-name] on purpose to trigger black-magic
```

## Testing
Expand Down Expand Up @@ -213,13 +220,15 @@ https://testandcode.com/116

<!-- Reference links -->

[black]: https://black.readthedocs.io/en/stable/
[doctest]: https://docs.python.org/3/library/doctest.html
[flake8]: https://flake8.pycqa.org/
[google-style-guide]: https://google.github.io/styleguide/pyguide.html
[isort]: https://pycqa.github.io/isort/
[mypy]: https://mypy.readthedocs.io/
[mypy-error-codes]: https://mypy.readthedocs.io/en/stable/error_code_list.html
[pre-commit]: https://pre-commit.com/
[pylint]: https://pylint.pycqa.org/
[ruff-formatter]: https://docs.astral.sh/ruff/formatter/
[ruff-linter]: https://docs.astral.sh/ruff/linter/
[ruff-rules]: https://docs.astral.sh/ruff/rules/
[sphinx]: https://www.sphinx-doc.org
[sphinx-autodoc]: https://www.sphinx-doc.org/en/master/usage/extensions/autodoc.html
[sphinx-napoleon]: https://sphinxcontrib-napoleon.readthedocs.io/en/latest/index.html#
Expand Down
15 changes: 6 additions & 9 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -376,10 +376,13 @@ line-length = 100 # It should be the same as in `tool.black.line-length` above
respect-gitignore = true
show-fixes = true
# show-source = true
target-version = 'py310'
target-version = 'py38'

[tool.ruff.format]
docstring-code-format = true
# Reevaluate once ruff adds support for single-line ellipsis in the stable style.
# Track: https://github.com/astral-sh/ruff/issues/8678)
preview = true

[tool.ruff.lint]
# # Rules sets:
Expand All @@ -393,14 +396,7 @@ docstring-code-format = true
# NPY: NumPy-specific rules
# RUF: Ruff-specific rules
ignore = [
'B008', # Do not perform function calls in argument defaults
# 'B028', # Consider replacing f"'{foo}'" with f"{foo!r}" # TODO: review
'B905', # B905 `zip()` without an explicit `strict=` parameter # TODO: review
# 'D1', # Public code object needs docstring
# 'E203', # Whitespace before ':' (black formatter breaks this sometimes)
'E501', # Line too long (using Bugbear's B950 warning)
'E701', # Multiple statements on one line, see https://github.com/psf/black/issues/3887
'RUF100'
'E501' # [line-too-long]
]
ignore-init-module-imports = true
select = ['E', 'F', 'I', 'B', 'A', 'T10', 'ERA', 'NPY', 'RUF']
Expand Down Expand Up @@ -455,6 +451,7 @@ split-on-trailing-comma = false
max-complexity = 15

[tool.ruff.lint.per-file-ignores]
"src/gt4py/cartesian/*" = ["RUF012"]
'src/gt4py/eve/extended_typing.py' = ['F401', 'F405']
'src/gt4py/next/__init__.py' = ['F401']

Expand Down
2 changes: 1 addition & 1 deletion src/gt4py/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,6 @@


if _sys.version_info >= (3, 10):
from . import next # noqa: A004
from . import next

__all__ += ["next"]
12 changes: 8 additions & 4 deletions src/gt4py/_core/definitions.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,21 +74,24 @@
BoolScalar: TypeAlias = Union[bool_, bool]
BoolT = TypeVar("BoolT", bound=BoolScalar)
BOOL_TYPES: Final[Tuple[type, ...]] = cast(
Tuple[type, ...], BoolScalar.__args__ # type: ignore[attr-defined]
Tuple[type, ...],
BoolScalar.__args__, # type: ignore[attr-defined]
)


IntScalar: TypeAlias = Union[int8, int16, int32, int64, int]
IntT = TypeVar("IntT", bound=IntScalar)
INT_TYPES: Final[Tuple[type, ...]] = cast(
Tuple[type, ...], IntScalar.__args__ # type: ignore[attr-defined]
Tuple[type, ...],
IntScalar.__args__, # type: ignore[attr-defined]
)


UnsignedIntScalar: TypeAlias = Union[uint8, uint16, uint32, uint64]
UnsignedIntT = TypeVar("UnsignedIntT", bound=UnsignedIntScalar)
UINT_TYPES: Final[Tuple[type, ...]] = cast(
Tuple[type, ...], UnsignedIntScalar.__args__ # type: ignore[attr-defined]
Tuple[type, ...],
UnsignedIntScalar.__args__, # type: ignore[attr-defined]
)


Expand All @@ -100,7 +103,8 @@
FloatingScalar: TypeAlias = Union[float32, float64, float]
FloatingT = TypeVar("FloatingT", bound=FloatingScalar)
FLOAT_TYPES: Final[Tuple[type, ...]] = cast(
Tuple[type, ...], FloatingScalar.__args__ # type: ignore[attr-defined]
Tuple[type, ...],
FloatingScalar.__args__, # type: ignore[attr-defined]
)


Expand Down
2 changes: 1 addition & 1 deletion src/gt4py/cartesian/__gtscript__.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

import sys

from gt4py.cartesian.gtscript import *
from gt4py.cartesian.gtscript import * # noqa: F403 [undefined-local-with-import-star]


sys.modules["__gtscript__"] = sys.modules["gt4py.cartesian.__gtscript__"]
17 changes: 17 additions & 0 deletions src/gt4py/cartesian/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,20 @@
type_hints,
)
from .stencil_object import StencilObject


__all__ = [
"typing",
"caching",
"cli",
"config",
"definitions",
"frontend",
"gt_cache_manager",
"gtscript",
"loader",
"stencil_builder",
"stencil_object",
"type_hints",
"StencilObject",
]
11 changes: 7 additions & 4 deletions src/gt4py/cartesian/backend/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,9 @@ def check_options(self, options: gt_definitions.BuildOptions) -> None:
unknown_options = set(options.backend_opts.keys()) - set(self.options.keys())
if unknown_options:
warnings.warn(
f"Unknown options '{unknown_options}' for backend '{self.name}'", RuntimeWarning
f"Unknown options '{unknown_options}' for backend '{self.name}'",
RuntimeWarning,
stacklevel=2,
)

def make_module(
Expand Down Expand Up @@ -409,8 +411,9 @@ def build_extension_module(

assert module_name == qualified_pyext_name

self.builder.with_backend_data(
{"pyext_module_name": module_name, "pyext_file_path": file_path}
)
self.builder.with_backend_data({
"pyext_module_name": module_name,
"pyext_file_path": file_path,
})

return module_name, file_path
Loading

0 comments on commit 6a19355

Please sign in to comment.