[style]: updates to coding guidelines and fixes for all QA errors after migrating to ruff

Author: egparedes

CODING_GUIDELINES.md

@@ -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.
 
@@ -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.
@@ -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
@@ -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#

src/gt4py/cartesian/__gtscript__.py

@@ -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__"]

src/gt4py/cartesian/__init__.py

@@ -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",
+]

src/gt4py/cartesian/backend/base.py

@@ -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(

src/gt4py/cartesian/backend/dace_backend.py

@@ -516,7 +516,7 @@ def _postprocess_dace_code(code_objects, is_gpu, builder):
                     break
             for i, line in enumerate(lines):
                 if "#include <dace/dace.h>" in line:
-                    cuda_code = [co.clean_code for co in code_objects if co.title == "CUDA"][0]
+                    cuda_code = next(co.clean_code for co in code_objects if co.title == "CUDA")
                     lines = lines[0:i] + cuda_code.split("\n") + lines[i + 1 :]
                     break
 

src/gt4py/cartesian/backend/pyext_builder.py

@@ -194,7 +194,7 @@ def build_pybind_ext(
     libraries: Optional[List[str]] = None,
     extra_compile_args: Optional[Union[List[str], Dict[str, List[str]]]] = None,
     extra_link_args: Optional[List[str]] = None,
-    build_ext_class: Type = None,
+    build_ext_class: Optional[Type] = None,
     verbose: bool = False,
     clean: bool = False,
 ) -> Tuple[str, str]: ...
@@ -211,7 +211,7 @@ def build_pybind_ext(
     libraries: Optional[List[str]] = None,
     extra_compile_args: Optional[Union[List[str], Dict[str, List[str]]]] = None,
     extra_link_args: Optional[List[str]] = None,
-    build_ext_class: Type = None,
+    build_ext_class: Optional[Type] = None,
     verbose: bool = False,
     clean: bool = False,
 ) -> Tuple[str, str]:
@@ -242,7 +242,6 @@ def build_pybind_ext(
         ext_modules=[py_extension],
         script_args=[
             "build_ext",
-            # "--parallel={}".format(gt_config.build_settings["parallel_jobs"]),
             "--build-temp={}".format(build_path),
             "--build-lib={}".format(build_path),
             "--force",
@@ -336,7 +335,7 @@ def build_pybind_cuda_ext(
 
 def _clean_build_flags(config_vars: Dict[str, str]) -> None:
     for key, value in config_vars.items():
-        if type(value) == str:
+        if isinstance(value, str):
             value = " " + value + " "
             for s in value.split(" "):
                 if (

src/gt4py/cartesian/frontend/defir_to_gtir.py

@@ -137,7 +137,7 @@ def visit_StencilDefinition(
     def _nested_list_dim(self, a: List) -> List[int]:
         if not isinstance(a, list):
             return []
-        return [len(a)] + self._nested_list_dim(a[0])
+        return [len(a), *self._nested_list_dim(a[0])]
 
     def visit_Assign(
         self, node: Assign, *, fields_decls: Dict[str, FieldDecl], **kwargs
@@ -181,7 +181,7 @@ def apply(cls, root, *, expected_dim: Tuple[int, ...], fields_decls: Dict[str, F
         # if the expression is just a scalar broadcast to the expected dimensions
         if not isinstance(result, list):
             result = functools.reduce(
-                lambda val, len: [val for _ in range(len)], reversed(expected_dim), result
+                lambda val, len_: [val for _ in range(len_)], reversed(expected_dim), result
             )
         return result
 

src/gt4py/cartesian/frontend/gtscript_frontend.py

@@ -211,15 +211,15 @@ def visit_BinOp(self, node: ast.BinOp) -> Union[gtscript.AxisIndex, nodes.AxisBo
         right = self.visit(node.right)
 
         if isinstance(node.op, ast.Add):
-            bin_op = lambda x, y: x + y  # noqa: E731
-            u_op = lambda x: x  # noqa: E731
+            bin_op = lambda x, y: x + y  # noqa: E731 [lambda-assignment]
+            u_op = lambda x: x  # noqa: E731 [lambda-assignment]
         elif isinstance(node.op, ast.Sub):
-            bin_op = lambda x, y: x - y  # noqa: E731
-            u_op = lambda x: -x  # noqa: E731
+            bin_op = lambda x, y: x - y  # noqa: E731 [lambda-assignment]
+            u_op = lambda x: -x  # noqa: E731 [lambda-assignment]
         elif isinstance(node.op, ast.Mult):
             if left.level != right.level or not isinstance(left.level, nodes.LevelMarker):
                 raise self.interval_error
-            bin_op = lambda x, y: x * y  # noqa: E731
+            bin_op = lambda x, y: x * y  # noqa: E731 [lambda-assignment]
             u_op = None
         else:
             raise GTScriptSyntaxError("Unexpected binary operator found in interval expression")
@@ -249,7 +249,7 @@ def visit_BinOp(self, node: ast.BinOp) -> Union[gtscript.AxisIndex, nodes.AxisBo
 
     def visit_UnaryOp(self, node: ast.UnaryOp) -> nodes.AxisBound:
         if isinstance(node.op, ast.USub):
-            op = lambda x: -x  # noqa: E731
+            op = lambda x: -x  # noqa: E731 [lambda-assignment]
         else:
             raise self.interval_error
 
@@ -417,9 +417,7 @@ def visit_Assign(self, node: ast.Assign):
         else:
             return self.generic_visit(node)
 
-    def visit_Call(  # Cyclomatic complexity too high
-        self, node: ast.Call, *, target_node=None
-    ):
+    def visit_Call(self, node: ast.Call, *, target_node=None):  # Cyclomatic complexity too high
         call_name = gt_meta.get_qualified_name_from_node(node.func)
 
         if call_name in self.call_stack:
@@ -461,10 +459,10 @@ def visit_Call(  # Cyclomatic complexity too high
                 if name not in call_args:
                     assert arg_infos[name] != nodes.Empty
                     call_args[name] = ast.Constant(value=arg_infos[name])
-        except Exception:
+        except Exception as ex:
             raise GTScriptSyntaxError(
                 message="Invalid call signature", loc=nodes.Location.from_ast_node(node)
-            )
+            ) from ex
 
         # Rename local names in subroutine to avoid conflicts with caller context names
         try:
@@ -601,7 +599,7 @@ def visit_If(self, node: ast.If):
             and node.test.func.id == "__INLINED"
             and len(node.test.args) == 1
         ):
-            warnings.warn(
+            warnings.warn(  # noqa: B028 [no-explicit-stacklevel]
                 f"stencil {self.stencil_name}, line {node.lineno}, column {node.col_offset}: compile-time if condition via __INLINED deprecated",
                 category=DeprecationWarning,
             )
@@ -1059,10 +1057,10 @@ def _eval_new_spatial_index(
         for index_node in index_nodes:
             try:
                 value = gt_meta.ast_eval(index_node, axis_context)
-            except Exception:
+            except Exception as ex:
                 raise GTScriptSyntaxError(
                     message="Could not evaluate axis shift expression.", loc=index_node
-                )
+                ) from ex
             if not isinstance(value, (gtscript.ShiftedAxis, gtscript.Axis)):
                 raise GTScriptSyntaxError(
                     message=f"Axis shift expression evaluated to unrecognized type {type(value)}.",

src/gt4py/cartesian/frontend/node_util.py

@@ -69,7 +69,7 @@ def generic_visit(self, node: Node, **kwargs):
         else:
             pass
 
-        for key, value in items:
+        for _, value in items:
             self._visit(value, **kwargs)
 
 
@@ -122,7 +122,7 @@ def iter_nodes_of_type(root_node: Node, node_type: Type) -> Generator[Node, None
     """Yield an iterator over the nodes of node_type inside root_node in DFS order."""
 
     def recurse(node: Node) -> Generator[Node, None, None]:
-        for key, value in iter_attributes(node):
+        for _, value in iter_attributes(node):
             if isinstance(node, collections.abc.Iterable):
                 if isinstance(node, collections.abc.Mapping):
                     children = node.values()

src/gt4py/cartesian/frontend/nodes.py

@@ -322,15 +322,6 @@ class ScalarLiteral(Literal):
     loc = attribute(of=Location, optional=True)
 
 
-# @attribclass
-# class TupleLiteral(Node):
-#     items = attribute(of=TupleOf[Expr])
-#
-#     @property
-#     def length(self):
-#         return len(self.items)
-
-
 @attribclass
 class BuiltinLiteral(Literal):
     value = attribute(of=Builtin)
@@ -593,12 +584,6 @@ class Statement(Node):
     pass
 
 
-# @attribclass
-# class ExprStmt(Statement):
-#     expr = attribute(of=Expr)
-#     loc = attribute(of=Location, optional=True)
-
-
 class Decl(Statement):
     pass
 
@@ -722,9 +707,6 @@ def is_single_index(self) -> bool:
         return self.start.level == self.end.level and self.start.offset == self.end.offset - 1
 
     def disjoint_from(self, other: "AxisInterval") -> bool:
-        # This made-up constant must be larger than any LevelMarker.offset used
-        DOMAIN_SIZE: int = 1000
-
         def get_offset(bound: AxisBound) -> int:
             return (
                 0 + bound.offset if bound.level == LevelMarker.START else sys.maxsize + bound.offset

src/gt4py/cartesian/gtc/dace/expansion/tasklet_codegen.py

@@ -225,7 +225,7 @@ def _visit_conditional(
             indent = " " * 4
         body_code = [line for block in self.visit(body, **kwargs) for line in block.split("\n")]
         body_code = [indent + b for b in body_code]
-        return "\n".join([mask_str] + body_code)
+        return "\n".join([mask_str, *body_code])
 
     def visit_MaskStmt(self, node: dcir.MaskStmt, **kwargs):
         return self._visit_conditional(cond=node.mask, body=node.body, keyword="if", **kwargs)

src/gt4py/cartesian/gtc/daceir.py

@@ -43,7 +43,7 @@ class Stmt(common.Stmt):
 
 
 class Axis(eve.StrEnum):
-    I = "I"  # noqa: E741 ambiguous variable name 'I'
+    I = "I"  # noqa: E741 [ambiguous-variable-name]
     J = "J"
     K = "K"
 

src/gt4py/cartesian/gtc/definitions.py

@@ -23,7 +23,7 @@
 class CartesianSpace:
     @enum.unique
     class Axis(enum.Enum):
-        I = 0  # noqa: E741  # Do not use variables named 'I', 'O', or 'l'
+        I = 0  # noqa: E741 [ambiguous-variable-name]
         J = 1
         K = 2
 

src/gt4py/cartesian/gtc/gtcpp/oir_to_gtcpp.py

@@ -17,7 +17,7 @@
 from dataclasses import dataclass, field
 from typing import Any, Callable, Dict, List, Set, Union, cast
 
-from devtools import debug  # noqa: F401
+from devtools import debug  # noqa: F401 [unused-import]
 from typing_extensions import Protocol
 
 from gt4py import eve

src/gt4py/cartesian/gtc/numpy/npir.py

@@ -22,7 +22,7 @@
 
 # --- Misc ---
 class AxisName(eve.StrEnum):
-    I = "I"  # noqa: E741 (ambiguous variable name)
+    I = "I"  # noqa: E741 [ambiguous-variable-name]
     J = "J"
     K = "K"
 

src/gt4py/cartesian/gtc/passes/gtir_definitive_assignment_analysis.py

@@ -74,6 +74,6 @@ def check(gtir_stencil_expr: gtir.Stencil) -> gtir.Stencil:
     """Execute definitive assignment analysis and warn on errors."""
     invalid_accesses = analyze(gtir_stencil_expr)
     for invalid_access in invalid_accesses:
-        warnings.warn(f"`{invalid_access.name}` may be uninitialized.")
+        warnings.warn(f"`{invalid_access.name}` may be uninitialized.", stacklevel=2)
 
     return gtir_stencil_expr

src/gt4py/cartesian/gtc/passes/oir_optimizations/horizontal_execution_merging.py

@@ -256,7 +256,7 @@ def first_has_horizontal_restriction() -> bool:
             or first_has_variable_access()
             or first_has_horizontal_restriction()
         ):
-            return [first] + self._merge(others, symtable, new_symbol_name, protected_fields)
+            return [first, *self._merge(others, symtable, new_symbol_name, protected_fields)]
 
         first_scalars = {decl.name for decl in first.declarations}
         writes = first_accesses.write_fields()

src/gt4py/cartesian/gtc/passes/oir_optimizations/vertical_loop_merging.py

@@ -38,7 +38,9 @@ def _mergeable(a: oir.VerticalLoop, b: oir.VerticalLoop) -> bool:
     def _merge(a: oir.VerticalLoop, b: oir.VerticalLoop) -> oir.VerticalLoop:
         sections = a.sections + b.sections
         if a.caches or b.caches:
-            warnings.warn("AdjacentLoopMerging pass removed previously declared caches")
+            warnings.warn(
+                "AdjacentLoopMerging pass removed previously declared caches", stacklevel=2
+            )
         return oir.VerticalLoop(
             loop_order=a.loop_order,
             sections=sections,

src/gt4py/cartesian/gtc/ufuncs.py

@@ -40,7 +40,7 @@
 not_equal: np.ufunc = np.not_equal
 logical_and: np.ufunc = np.logical_and
 logical_or: np.ufunc = np.logical_or
-abs: np.ufunc = np.abs  # noqa: A001  # shadowing abs builtin
+abs: np.ufunc = np.abs  # noqa: A001 [builtin-variable-shadowing]
 minimum: np.ufunc = np.minimum
 maximum: np.ufunc = np.maximum
 remainder: np.ufunc = np.remainder