From 638611f760075bebb68eba1e21296940dbc3cb04 Mon Sep 17 00:00:00 2001 From: Benjamin Schubert Date: Sat, 30 Dec 2023 15:01:43 +0000 Subject: [PATCH] Configure ruff for this project and fix problems Most notable are: - require boolean arguments to be passed as keywords Otherwise it can't be quite confusing and ordering becomes less clear. Overall this reduces errors - Ensure all imports used for typechecking are guarded by TYPE_CHECKING --- .github/scripts/summary.py | 9 ++-- docs/_extensions/cleanup_signatures.py | 1 + docs/_extensions/execute.py | 10 ++-- docs/_extensions/styles.py | 4 +- docs/conf.py | 2 +- dwasfile.py | 5 +- pyproject.toml | 72 +++++++++++++++++++++++++ src/dwas/__main__.py | 47 ++++++++++------ src/dwas/_config.py | 1 + src/dwas/_frontend.py | 4 +- src/dwas/_inspect.py | 2 +- src/dwas/_io.py | 12 +++-- src/dwas/_logging.py | 29 +++++----- src/dwas/_pipeline.py | 74 ++++++++++++++++---------- src/dwas/_runners.py | 24 ++++++--- src/dwas/_steps/handlers.py | 25 +++++---- src/dwas/_steps/parametrize.py | 11 ++-- src/dwas/_steps/registration.py | 2 +- src/dwas/_steps/steps.py | 12 ++--- src/dwas/predefined/__init__.py | 3 +- src/dwas/predefined/_package.py | 2 +- src/dwas/predefined/_pylint.py | 2 +- src/dwas/predefined/_sphinx.py | 1 + tests/_utils.py | 5 +- tests/conftest.py | 2 +- tests/predefined/mixins.py | 14 +++-- tests/test_pipeline.py | 25 +++++---- tests/test_registration.py | 4 +- 28 files changed, 277 insertions(+), 127 deletions(-) diff --git a/.github/scripts/summary.py b/.github/scripts/summary.py index 0c24d0e..d243065 100644 --- a/.github/scripts/summary.py +++ b/.github/scripts/summary.py @@ -1,3 +1,4 @@ +# ruff: noqa:D100,D101,D102,D103,D105 import argparse import logging from dataclasses import InitVar, dataclass, field @@ -34,7 +35,7 @@ def from_junit(cls, tree: ElementTree.Element) -> "TestCase": elif child.tag == "skipped": state = "skipped" else: - assert False, f"unexpected tag: {child.tag}" + raise AssertionError(f"unexpected tag: {child.tag}") summary = child.attrib["message"].replace("\n", "
") @@ -136,7 +137,9 @@ def main() -> None: testsuites = [ TestSuite.from_junit(testsuite) for file in files - for testsuite in ElementTree.parse(file).findall("testsuite") + for testsuite in ElementTree.parse(file).findall( # noqa: S314 + "testsuite" + ) ] summary = tabulate( @@ -168,7 +171,7 @@ def main() -> None: errors = get_failures_and_errors(testsuites) - print( + print( # noqa: T201 f"""\ ## Test suites diff --git a/docs/_extensions/cleanup_signatures.py b/docs/_extensions/cleanup_signatures.py index edadeb3..153faa4 100644 --- a/docs/_extensions/cleanup_signatures.py +++ b/docs/_extensions/cleanup_signatures.py @@ -4,6 +4,7 @@ from sphinx.ext.autodoc import Options +# ruff: noqa: ARG001 def cleanup_signatures( # pylint: disable=unused-argument app: Sphinx, what: str, diff --git a/docs/_extensions/execute.py b/docs/_extensions/execute.py index 3ab1c06..21d4040 100644 --- a/docs/_extensions/execute.py +++ b/docs/_extensions/execute.py @@ -11,7 +11,7 @@ from sphinx.errors import ExtensionError -class execute(nodes.Element): # pylint: disable=invalid-name +class execute(nodes.Element): # pylint: disable=invalid-name # noqa: N801 pass @@ -20,7 +20,10 @@ class ExecuteDirective(rst.Directive): final_argument_whitespace = True required_arguments = 1 - option_spec = {"returncode": nonnegative_int, "cwd": unchanged} + option_spec = { # noqa: RUF012 + "returncode": nonnegative_int, + "cwd": unchanged, + } def run(self) -> List[nodes.Element]: env = self.state.document.settings.env @@ -40,7 +43,8 @@ def run(self) -> List[nodes.Element]: def run_programs( - app: Sphinx, doctree: document # pylint: disable=unused-argument + app: Sphinx, # pylint: disable=unused-argument # noqa: ARG001 + doctree: document, ) -> None: env = os.environ.copy() # Ensure we always have colors set for the output diff --git a/docs/_extensions/styles.py b/docs/_extensions/styles.py index 2a72254..8b80198 100644 --- a/docs/_extensions/styles.py +++ b/docs/_extensions/styles.py @@ -15,12 +15,12 @@ class AnsiMonokaiStyle(monokai.MonokaiStyle): - styles = dict(monokai.MonokaiStyle.styles) + styles = dict(monokai.MonokaiStyle.styles) # noqa: RUF012 styles.update(color_tokens(fg_colors, bg_colors)) styles[pygments.token.Token.Color.Faint.Cyan] = "#0867AC" class AnsiDefaultStyle(default.DefaultStyle): - styles = dict(default.DefaultStyle.styles) + styles = dict(default.DefaultStyle.styles) # noqa: RUF012 styles.update(color_tokens(fg_colors, bg_colors)) styles[pygments.token.Token.Color.Faint.Cyan] = "#0867AC" diff --git a/docs/conf.py b/docs/conf.py index 9874a7b..76ac2ff 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -27,7 +27,7 @@ release = _project_metadata["Version"] author = _project_metadata["Author-email"] # pylint: disable=redefined-builtin -copyright = "2022, Benjamin Schubert" +copyright = "2022, Benjamin Schubert" # noqa: A001 html_context = { "github_user": "BenjaminSchubert", diff --git a/dwasfile.py b/dwasfile.py index 2454589..adf4285 100644 --- a/dwasfile.py +++ b/dwasfile.py @@ -1,3 +1,6 @@ +""" +Contains the dwas configuration for this project. +""" from pathlib import Path import dwas @@ -226,5 +229,5 @@ "ci", ["docs", "format-check", "lint", "coverage", "twine:check"], "Run all checks that are run on CI", - False, + run_by_default=False, ) diff --git a/pyproject.toml b/pyproject.toml index bfddcdd..b55d000 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -124,6 +124,8 @@ disable = [ "missing-class-docstring", "missing-function-docstring", "missing-module-docstring", + # Taken care of by ruff + "broad-exception-caught", ] [tool.pylint.variables] @@ -133,7 +135,77 @@ disable = [ # Ruff [tool.ruff] target-version = "py38" +select = ["ALL"] +ignore = [ + ## + # Typing + "ANN101", # Don't annotate 'self' + "ANN102", # Don't annotate 'cls' + "ANN401", # Allow typing with ANY + ## + # Format + "COM812", # Don't put trailing commas everywhere + "E501", # line length is handled by black + ## + # Docs + "D200", + "D203", + "D212", + "D404", + ## + # Stylistic decisions + "PLR0911", # pylint too many return statements + "PLR0912", # pylint too many branches + "PLR0913", # pylint too many arguments + "PLR0915", # pylint too many statements + "N818", # we name exception as Exception not Error + "FIX", # We want to put fixmes longterm + "TD", # We want to be able to add todos without issu links + "INP001", # Not every .py file is part of a package + "PT004", # Don't add `_` in front of fixtures returning nothing + "S101", # Allow asserts + "TID252", # We want relative imports + "PLR2004", # Disable magic numbers check + # Exception handling + "EM101", + "EM102", + "RSE102", + "TRY002", + "TRY003", + # We are not doing cryptographic stuff + "S311", + # This is buggy and doesnt' say how to validate + "S603", + # We want to be able tor rely on PATH and not hardcode binaries + "S607", + + # FIXME: re-enable future annotations + "FA100", + # FIXME: re-enable upgrade rules + "UP", +] +flake8-pytest-style.parametrize-values-type = "tuple" +flake8-pytest-style.parametrize-values-row-type = "tuple" +flake8-pytest-style.fixture-parentheses = false +lint.isort.known-first-party = ["dwas"] + +[tool.ruff.lint.per-file-ignores] +"docs/*" = ["D"] +"tests/*" = [ + # No need to type annotate tests + "ANN", + # No need to document tests + "D100", + "D101", + "D102", + "D103", + "D104", + # too many arguments, acceptable for tests + "PLR0913", + # Allow prints in tests + "T201", +] ## diff --git a/src/dwas/__main__.py b/src/dwas/__main__.py index 32ecdee..b75b133 100644 --- a/src/dwas/__main__.py +++ b/src/dwas/__main__.py @@ -1,3 +1,4 @@ +# ruff: noqa:D100,D101,D102,D103 import importlib.util import logging import os @@ -27,10 +28,10 @@ class _SplitAppendAction(_AppendAction): def __call__( self, - parser: ArgumentParser, + parser: ArgumentParser, # noqa:ARG002 namespace: Namespace, values: Any, - option_string: Optional[str] = None, + option_string: Optional[str] = None, # noqa:ARG002 ) -> None: items = getattr(namespace, self.dest, None) if items is None: @@ -48,9 +49,9 @@ class BooleanOptionalAction(Action): # This can be replaced once we drop support for python3.8 def __call__( self, - parser: ArgumentParser, + parser: ArgumentParser, # noqa:ARG002 namespace: Namespace, - values: Any, + values: Any, # noqa:ARG002 option_string: Optional[str] = None, ) -> None: assert option_string is not None @@ -275,8 +276,9 @@ def _execute_pipeline( config: Config, pipeline_config: str, steps_parameters: List[str], - only_selected_step: bool, except_steps: Optional[List[str]], + *, + only_selected_step: bool, clean: bool, list_only: bool, list_dependencies: bool, @@ -296,11 +298,19 @@ def _execute_pipeline( if list_only or list_dependencies: pipeline.list_all_steps( - steps, except_steps, only_selected_step, list_dependencies + steps, + except_steps, + only_selected_steps=only_selected_step, + show_dependencies=list_dependencies, ) return - pipeline.execute(steps, except_steps, only_selected_step, clean=clean) + pipeline.execute( + steps, + except_steps, + only_selected_steps=only_selected_step, + clean=clean, + ) @_io.instrument_streams() @@ -318,13 +328,16 @@ def main(sys_args: Optional[List[str]] = None) -> None: verbosity, args.colors, args.jobs, - args.skip_missing_interpreters, - args.no_setup, - args.setup_only, - args.fail_fast, + skip_missing_interpreters=args.skip_missing_interpreters, + skip_setup=args.no_setup, + skip_run=args.setup_only, + fail_fast=args.fail_fast, ) setup_logging( - logging.INFO - 10 * verbosity, config.colors, _io.STDERR, _io.LOG_FILE + logging.INFO - 10 * verbosity, + _io.STDERR, + _io.LOG_FILE, + colors=config.colors, ) log_path = config.log_path / "main.log" @@ -334,18 +347,18 @@ def main(sys_args: Optional[List[str]] = None) -> None: config, args.config, args.steps_parameters, - args.only_steps, args.except_steps, - args.clean, - args.list_only, - args.list_dependencies, + only_selected_step=args.only_steps, + clean=args.clean, + list_only=args.list_only, + list_dependencies=args.list_dependencies, ) except BaseDwasException as exc: if config.verbosity >= 1 and not isinstance( exc, FailedPipelineException ): LOGGER.debug(exc, exc_info=exc) - LOGGER.error("%s", exc) + LOGGER.error("%s", exc) # noqa: TRY400 we don't want the traceback raise SystemExit(exc.exit_code) from exc finally: LOGGER.info("Logs can be found at %s", log_path) diff --git a/src/dwas/_config.py b/src/dwas/_config.py index 114d2a0..cc34c1e 100644 --- a/src/dwas/_config.py +++ b/src/dwas/_config.py @@ -127,6 +127,7 @@ def __init__( verbosity: int, colors: Optional[bool], n_jobs: int, + *, skip_missing_interpreters: bool, skip_setup: bool, skip_run: bool, diff --git a/src/dwas/_frontend.py b/src/dwas/_frontend.py index d5bff82..121f358 100644 --- a/src/dwas/_frontend.py +++ b/src/dwas/_frontend.py @@ -125,7 +125,7 @@ def _refresh(self) -> None: previous_summary_last_line_length = 0 previous_line_length = None - def refresh(skip_summary: bool = False) -> None: + def refresh(*, skip_summary: bool = False) -> None: nonlocal previous_summary_height nonlocal previous_summary_last_line_length nonlocal previous_line_length @@ -183,4 +183,4 @@ def refresh(skip_summary: bool = False) -> None: self._stop.wait(0.5) refresh() - refresh(True) + refresh(skip_summary=True) diff --git a/src/dwas/_inspect.py b/src/dwas/_inspect.py index c7d2045..d2b75fd 100644 --- a/src/dwas/_inspect.py +++ b/src/dwas/_inspect.py @@ -15,6 +15,6 @@ def get_location(obj: Any) -> str: def get_name(func: Callable[..., Any]) -> str: if inspect.isfunction(func): return func.__name__ - actual_func = getattr(func, "__call__") + actual_func = func.__call__ # type: ignore[operator] func_name = actual_func.__name__ return f"{func.__class__.__name__}.{func_name}" diff --git a/src/dwas/_io.py b/src/dwas/_io.py index da97827..9a06624 100644 --- a/src/dwas/_io.py +++ b/src/dwas/_io.py @@ -24,7 +24,7 @@ class NoOpWriter(io.TextIOWrapper): def __init__(self) -> None: pass - def read(self, size: Optional[int] = None) -> str: + def read(self, size: Optional[int] = None) -> str: # noqa: ARG002 raise io.UnsupportedOperation("Can't read from a noopwriter") def write(self, data: str) -> int: @@ -41,7 +41,7 @@ def __init__( ) -> None: self._writer = writer - def read(self, size: Optional[int] = None) -> str: + def read(self, size: Optional[int] = None) -> str: # noqa: ARG002 raise io.UnsupportedOperation("can't read from a memorypipe") def write(self, data: str) -> int: @@ -52,7 +52,7 @@ def flush(self) -> None: class PipePlexer: - def __init__(self, write_on_flush: bool = True) -> None: + def __init__(self, *, write_on_flush: bool = True) -> None: self.stderr = MemoryPipe(self) self.stdout = MemoryPipe(self) @@ -63,7 +63,9 @@ def write(self, stream: MemoryPipe, data: str) -> int: self._buffer.append((stream, data)) return len(data) - def flush(self, force_write: bool = False) -> Optional[int]: + def flush( + self, force_write: bool = False # noqa:FBT001,FBT002 + ) -> Optional[int]: line = None if self._write_on_flush or force_write: @@ -94,7 +96,7 @@ def __init__( self._var = var self._log_var = log_var - def read(self, size: Optional[int] = None) -> str: + def read(self, size: Optional[int] = None) -> str: # noqa: ARG002 raise io.UnsupportedOperation("can't read from a memorypipe") def write(self, data: str) -> int: diff --git a/src/dwas/_logging.py b/src/dwas/_logging.py index d2cbce6..9ac41a3 100644 --- a/src/dwas/_logging.py +++ b/src/dwas/_logging.py @@ -1,6 +1,6 @@ import logging from contextvars import ContextVar -from types import TracebackType +from types import MappingProxyType, TracebackType from typing import Any, Optional, TextIO, Tuple, Type, Union, cast from colorama import Back, Fore, Style, init @@ -9,13 +9,18 @@ class ColorFormatter(logging.Formatter): - COLOR_MAPPING = { - logging.DEBUG: Fore.CYAN, - logging.INFO: "", - logging.WARN: Fore.YELLOW, - logging.ERROR: Fore.RED + Style.BRIGHT, - logging.FATAL: Back.RED + Fore.WHITE + Style.BRIGHT, - } + # We need to follow camel case style + # ruff: noqa: N802 + + COLOR_MAPPING = MappingProxyType( + { + logging.DEBUG: Fore.CYAN, + logging.INFO: "", + logging.WARN: Fore.YELLOW, + logging.ERROR: Fore.RED + Style.BRIGHT, + logging.FATAL: Back.RED + Fore.WHITE + Style.BRIGHT, + } + ) def formatMessage(self, record: logging.LogRecord) -> str: cast(Any, record).level_color = self.COLOR_MAPPING[record.levelno] @@ -29,10 +34,7 @@ def formatException( ], ) -> str: output = super().formatException(ei) - output = f"{Fore.CYAN}\ndwas > " + "\ndwas > ".join( - output.splitlines() - ) - return output + return f"{Fore.CYAN}\ndwas > " + "\ndwas > ".join(output.splitlines()) class NoColorFormatter(logging.Formatter): @@ -55,9 +57,10 @@ def stream(self, value: ContextVar[TextIO]) -> None: def setup_logging( level: int, - colors: bool, tty_output: ContextVar[TextIO], log_file: ContextVar[TextIO], + *, + colors: bool, ) -> None: nocolor_formatter = NoColorFormatter( fmt="dwas > [%(levelname)s] %(message)s" diff --git a/src/dwas/_pipeline.py b/src/dwas/_pipeline.py index 0ad427c..8a5b4cb 100644 --- a/src/dwas/_pipeline.py +++ b/src/dwas/_pipeline.py @@ -9,17 +9,21 @@ from contextvars import ContextVar, copy_context from datetime import timedelta from subprocess import CalledProcessError -from types import FrameType -from typing import Callable, Dict, Generator, List, Optional, Set, Tuple, cast +from typing import ( + TYPE_CHECKING, + Callable, + Dict, + Generator, + List, + Optional, + Set, + Tuple, + cast, +) from colorama import Fore, Style -from dwas._steps.handlers import BaseStepHandler, StepGroupHandler, StepHandler -from dwas._steps.parametrize import extract_parameters -from dwas._steps.steps import Step - from . import _io -from ._config import Config from ._exceptions import ( DuplicateStepException, FailedPipelineException, @@ -28,9 +32,18 @@ ) from ._frontend import Frontend, StepSummary from ._scheduler import JobResult, Resolver, Scheduler +from ._steps.handlers import BaseStepHandler, StepGroupHandler, StepHandler +from ._steps.parametrize import extract_parameters from ._subproc import ProcessManager from ._timing import format_timedelta, get_timedelta_since +if TYPE_CHECKING: + from types import FrameType + + from ._config import Config + from ._steps.steps import Step + + LOGGER = logging.getLogger(__name__) _PIPELINE = ContextVar["Pipeline"]("pipeline") @@ -155,10 +168,11 @@ def _resolve_parameters( name, self, all_created, all_run_by_default, description ) - def _build_graph( + def _build_graph( # noqa: C901 self, steps: Optional[List[str]] = None, except_steps: Optional[List[str]] = None, + *, only_selected_steps: bool = False, ) -> Dict[str, List[str]]: # we should refactor at some point @@ -180,10 +194,9 @@ def expand(steps: List[str]) -> Set[str]: return expanded - if except_steps is None: - except_steps_set = set() - else: - except_steps_set = expand(except_steps) + except_steps_set = ( + set() if except_steps is None else expand(except_steps) + ) if steps is None: steps = [ @@ -233,11 +246,12 @@ def compute_replacement(requirements: List[str]) -> List[str]: return replacements for step in except_steps_set: - if step not in except_replacements: - # The step might not be in the graph, if it is not depended - # upon by anything else - if deps := graph.get(step): - except_replacements[step] = compute_replacement(deps) + # The step might not be in the graph, if it is not depended + # upon by anything else + if step not in except_replacements and ( + deps := graph.get(step) + ): + except_replacements[step] = compute_replacement(deps) graph = { key: [ @@ -269,7 +283,7 @@ def compute_only_replacement( return replacements - for step in graph.keys(): + for step in graph: if step not in only_replacements: only_replacements[step] = compute_only_replacement( step, graph[step] @@ -287,13 +301,16 @@ def execute( self, steps: Optional[List[str]], except_steps: Optional[List[str]], + *, only_selected_steps: bool, clean: bool, ) -> None: # pylint: disable=too-many-locals start_time = time.monotonic() - graph = self._build_graph(steps, except_steps, only_selected_steps) + graph = self._build_graph( + steps, except_steps, only_selected_steps=only_selected_steps + ) resolver = Resolver(graph) scheduler = resolver.get_scheduler() steps_in_order = resolver.order() @@ -343,6 +360,7 @@ def list_all_steps( self, steps: Optional[List[str]] = None, except_steps: Optional[List[str]] = None, + *, only_selected_steps: bool = False, show_dependencies: bool = False, ) -> None: @@ -351,7 +369,9 @@ def list_all_steps( self._build_graph(list(self.steps.keys())) ).order() selected_steps = Resolver( - self._build_graph(steps, except_steps, only_selected_steps) + self._build_graph( + steps, except_steps, only_selected_steps=only_selected_steps + ) ).order() step_infos = [] @@ -385,9 +405,9 @@ def list_all_steps( indicator = "-" if self.config.verbosity > 0 and description: - description = f"\t[{Fore.BLUE}{Style.NORMAL}{description}{Style.RESET_ALL}{style}]" + desc = f"\t[{Fore.BLUE}{Style.NORMAL}{description}{Style.RESET_ALL}{style}]" else: - description = "" + desc = "" LOGGER.info( "\t%s%s %-*s%-*s%s", @@ -397,7 +417,7 @@ def list_all_steps( step, max_dependencies_length, dependencies, - description, + desc, ) def _execute( @@ -461,7 +481,7 @@ def _handle_step_result( ) -> None: try: result.result() - except Exception as exc: # pylint: disable=broad-exception-caught + except Exception as exc: # noqa:BLE001 if ( isinstance(exc, UnavailableInterpreterException) and self.config.skip_missing_interpreters @@ -485,7 +505,7 @@ def _handle_step_result( ) else None ) - LOGGER.error( + LOGGER.error( # noqa: TRY400, we want more control than .exception "Step %s failed: %s", name, self._format_exception(exc), @@ -630,7 +650,7 @@ def _display_slowest_dependency_chain( total_slowest_time = timedelta() # Use the graph here, to display them in topological order - for step in graph.keys(): + for step in graph: if step not in total_time_per_step: continue @@ -678,7 +698,7 @@ def compute_chain(step: str) -> Tuple[List[str], timedelta]: return total_time_per_step[step] - for step in results.keys(): + for step in results: compute_chain(step) return total_time_per_step diff --git a/src/dwas/_runners.py b/src/dwas/_runners.py index 7db2492..97f8430 100644 --- a/src/dwas/_runners.py +++ b/src/dwas/_runners.py @@ -3,23 +3,28 @@ import logging import os import shutil -import subprocess import sys from contextlib import suppress -from pathlib import Path -from typing import Dict, List, Optional, cast +from typing import TYPE_CHECKING, Dict, List, Optional, cast from virtualenv import session_via_cli -from virtualenv.run.session import Session from . import _io -from ._config import Config from ._exceptions import ( CommandNotFoundException, CommandNotInEnvironment, UnavailableInterpreterException, ) -from ._subproc import ProcessManager + +if TYPE_CHECKING: + import subprocess + from pathlib import Path + + from virtualenv.run.session import Session + + from ._config import Config + from ._subproc import ProcessManager + LOGGER = logging.getLogger(__name__) @@ -122,11 +127,14 @@ def run( str | bytes | os.PathLike[str] | os.PathLike[bytes] ] = None, env: Optional[Dict[str, str]] = None, + *, external_command: bool = False, silent_on_success: bool = False, ) -> subprocess.CompletedProcess[None]: env = self._merge_env(self._config, env) - self._validate_command(command[0], external_command, env) + self._validate_command( + command[0], env, external_command=external_command + ) return self._proc_manager.run( command, @@ -151,7 +159,7 @@ def _merge_env( return env def _validate_command( - self, command: str, external_command: bool, env: Dict[str, str] + self, command: str, env: Dict[str, str], *, external_command: bool ) -> None: cmd = shutil.which(command, path=env["PATH"]) diff --git a/src/dwas/_steps/handlers.py b/src/dwas/_steps/handlers.py index adfa878..7fd0e05 100644 --- a/src/dwas/_steps/handlers.py +++ b/src/dwas/_steps/handlers.py @@ -5,12 +5,10 @@ import logging import os import shutil -import subprocess from abc import ABC, abstractmethod from contextlib import suppress from typing import TYPE_CHECKING, Any, Dict, List, Optional -from .._config import Config from .._dependency_injection import call_with_parameters from .._exceptions import BaseDwasException from .._runners import VenvRunner @@ -24,6 +22,9 @@ ) if TYPE_CHECKING: + import subprocess + + from .._config import Config from .._pipeline import Pipeline @@ -140,7 +141,9 @@ def get_artifacts(self, key: str) -> List[Any]: [ # Pylint check here is wrong, it's still an instance of our class # pylint: disable=protected-access - self._pipeline.get_step(requirement)._get_artifacts(key) + self._pipeline.get_step( # noqa: SLF001 + requirement + )._get_artifacts(key) for requirement in self.requires ] ) @@ -184,7 +187,9 @@ def execute(self) -> None: for requirement in self.requires: # Pylint check here is wrong, it's still an instance of our class # pylint: disable=protected-access - self._pipeline.get_step(requirement)._execute_dependent_setup(self) + self._pipeline.get_step( # noqa: SLF001 + requirement + )._execute_dependent_setup(self) call_with_parameters(self._func, self.parameters.copy()) @@ -208,7 +213,7 @@ def _execute_dependent_setup( self._step_runner, # Pylint check here is wrong, it's still an instance of our class # pylint: disable=protected-access - current_step._step_runner, + current_step._step_runner, # noqa: SLF001 ) def _get_artifacts(self, key: str) -> List[Any]: @@ -276,9 +281,9 @@ def _execute_dependent_setup( for requirement in self.requires: # Pylint check here is wrong, it's still an instance of our class # pylint: disable=protected-access - self._pipeline.get_step(requirement)._execute_dependent_setup( - current_step - ) + self._pipeline.get_step( # noqa: SLF001 + requirement + )._execute_dependent_setup(current_step) def _get_artifacts(self, key: str) -> List[Any]: return list( @@ -286,7 +291,9 @@ def _get_artifacts(self, key: str) -> List[Any]: [ # Pylint check here is wrong, it's still an instance of our class # pylint: disable=protected-access - self._pipeline.get_step(requirement)._get_artifacts(key) + self._pipeline.get_step( # noqa: SLF001 + requirement + )._get_artifacts(key) for requirement in self.requires ] ) diff --git a/src/dwas/_steps/parametrize.py b/src/dwas/_steps/parametrize.py index c6f67e8..74aaee7 100644 --- a/src/dwas/_steps/parametrize.py +++ b/src/dwas/_steps/parametrize.py @@ -68,14 +68,15 @@ def as_dict(self) -> Dict[str, Any]: # pylint: disable=protected-access @classmethod def merge(cls, param1: "Parameter", param2: "Parameter") -> "Parameter": + # ruff: noqa: SLF001 if param1.id == "": id_ = param2.id elif param2.id == "": id_ = param1.id else: - id_ = ",".join([param1.id, param2.id]) + id_ = f"{param1.id},{param2.id}" - for key in param2._parameters.keys(): + for key in param2._parameters: if key in param1._parameters: raise ValueError(key) @@ -84,7 +85,7 @@ def merge(cls, param1: "Parameter", param2: "Parameter") -> "Parameter": return cls(id_, joined_parameters) - def __eq__(self, other: Any) -> bool: + def __eq__(self, other: object) -> bool: if not isinstance(other, type(self)): return NotImplemented return self.id == other.id and self._parameters == other._parameters @@ -223,8 +224,8 @@ def _apply(func: T) -> T: ids = [None] * len(args_values) current_parameters = [ - Parameter(id, dict(zip(arg_names, args_values))) - for id, args_values in zip(ids, args_values) + Parameter(id_, dict(zip(arg_names, args_values))) + for id_, args_values in zip(ids, args_values) ] old_parameters = getattr(func, _PARAMETERS, []) diff --git a/src/dwas/_steps/registration.py b/src/dwas/_steps/registration.py index c2324b9..8ae2184 100644 --- a/src/dwas/_steps/registration.py +++ b/src/dwas/_steps/registration.py @@ -206,7 +206,7 @@ def register_managed_step( def install(step: StepRunner, dependencies: str) -> None: step.install(*dependencies) - setattr(func, "setup", install) + func.setup = install # type: ignore[attr-defined] if dependencies is not None: func = parametrize("dependencies", [dependencies])(func) diff --git a/src/dwas/_steps/steps.py b/src/dwas/_steps/steps.py index 6dc4082..1167018 100644 --- a/src/dwas/_steps/steps.py +++ b/src/dwas/_steps/steps.py @@ -3,9 +3,7 @@ from __future__ import annotations -import os -import subprocess -from pathlib import Path +from pathlib import Path # noqa: TCH003 required for Sphinx from typing import ( TYPE_CHECKING, Any, @@ -17,9 +15,11 @@ runtime_checkable, ) -from .._config import Config - if TYPE_CHECKING: + import os + import subprocess + + from .._config import Config from .handlers import StepHandler @@ -94,7 +94,7 @@ def __call__(self, *args: Any, **kwargs: Any) -> None: For passing other arguments, see :py:func:`dwas.parametrize` and :py:func:`dwas.set_defaults`. - """ + """ # noqa: D401 @runtime_checkable diff --git a/src/dwas/predefined/__init__.py b/src/dwas/predefined/__init__.py index f35c8e0..43dbe95 100644 --- a/src/dwas/predefined/__init__.py +++ b/src/dwas/predefined/__init__.py @@ -1,6 +1,5 @@ """ -This module contains predefined steps that are provided in order to help reduce -duplication. +This module contains common predefined steps to help reduce duplication. .. note:: diff --git a/src/dwas/predefined/_package.py b/src/dwas/predefined/_package.py index b71065c..c4eb9c7 100644 --- a/src/dwas/predefined/_package.py +++ b/src/dwas/predefined/_package.py @@ -61,7 +61,7 @@ def setup_dependent( silent_on_success=current_step.config.verbosity < 2, ) - def __call__(self, step: StepRunner, isolate: bool) -> None: + def __call__(self, step: StepRunner, *, isolate: bool) -> None: with suppress(FileNotFoundError): shutil.rmtree(step.cache_path) diff --git a/src/dwas/predefined/_pylint.py b/src/dwas/predefined/_pylint.py index 5d246c7..8aa5baf 100644 --- a/src/dwas/predefined/_pylint.py +++ b/src/dwas/predefined/_pylint.py @@ -27,7 +27,7 @@ def __call__( if step.config.colors and not [ p for p in additional_arguments - if p.startswith("--output-format") or p.startswith("-f") + if p.startswith(("--output-format", "-f")) ]: additional_arguments.append("--output-format=colorized") diff --git a/src/dwas/predefined/_sphinx.py b/src/dwas/predefined/_sphinx.py index 9fc2016..71c4cd9 100644 --- a/src/dwas/predefined/_sphinx.py +++ b/src/dwas/predefined/_sphinx.py @@ -30,6 +30,7 @@ def __call__( builder: str, sourcedir: Union[Path, str], output: Optional[Union[Path, str]], + *, warning_as_error: bool, ) -> None: if step.config.verbosity == -2: diff --git a/tests/_utils.py b/tests/_utils.py index deb3dc9..97390be 100644 --- a/tests/_utils.py +++ b/tests/_utils.py @@ -51,8 +51,7 @@ def isolated_logging() -> Iterator[None]: def using_project(project: str) -> Callable[[_T], _T]: def wrapper(func): func = pytest.mark.project(TESTS_PATH / project)(func) - func = pytest.mark.usefixtures("project")(func) - return func + return pytest.mark.usefixtures("project")(func) return wrapper @@ -67,7 +66,7 @@ class Result: @isolated_context def execute(args: List[str], expected_status: int = 0) -> Result: """ - Runs dwas in an isolated context and returns the result from the run. + Run dwas in an isolated context and returns the result from the run. In most cases, you'll want to use the below `cli` instead, which will take care of building the cli correctly for you. diff --git a/tests/conftest.py b/tests/conftest.py index 517f8e7..1583499 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -25,7 +25,7 @@ def pytest_collection_modifyitems(items): for item in items: try: item.path.relative_to(predefined_tests_path) - except ValueError: + except ValueError: # noqa:PERF203 continue else: item.add_marker(pytest.mark.predefined) diff --git a/tests/predefined/mixins.py b/tests/predefined/mixins.py index 3f95eb8..4a62ad3 100644 --- a/tests/predefined/mixins.py +++ b/tests/predefined/mixins.py @@ -10,6 +10,12 @@ class BaseStepTest(ABC): + @abstractmethod + def expected_output(self) -> str: + """ + Get part of the expected output for the run. + """ + @pytest.fixture(scope="module") def cache_path(self, tmp_path_factory): return tmp_path_factory.mktemp("cache") @@ -20,7 +26,7 @@ def test_runs_successfully(self, cache_path, expected_output): assert expected_output in result.stdout @pytest.mark.parametrize( - "enable_colors", [True, False], ids=["colors", "no-colors"] + "enable_colors", (True, False), ids=["colors", "no-colors"] ) @pytest.mark.usefixtures("project") def test_respects_color_settings(self, cache_path, enable_colors): @@ -61,7 +67,7 @@ def valid_file(self) -> str: def cache_path(self, tmp_path_factory): return tmp_path_factory.mktemp("cache") - def _make_project(self, path: Path, valid: bool = True) -> None: + def _make_project(self, path: Path, *, valid: bool = True) -> None: path.joinpath("dwasfile.py").write_text(self.dwasfile) token_file = path.joinpath("src/token.py") @@ -72,14 +78,14 @@ def _make_project(self, path: Path, valid: bool = True) -> None: token_file.write_text(self.invalid_file) @pytest.mark.parametrize( - "valid", [True, False], ids=["valid-project", "invalid-project"] + "valid", (True, False), ids=["valid-project", "invalid-project"] ) def test_simple_behavior(self, cache_path, tmp_path, valid): self._make_project(tmp_path, valid=valid) cli(cache_path=cache_path, expected_status=0 if valid else 1) @pytest.mark.parametrize( - "enable_colors", [True, False], ids=["colors", "no-colors"] + "enable_colors", (True, False), ids=["colors", "no-colors"] ) def test_respects_color_settings( self, cache_path, tmp_path, enable_colors diff --git a/tests/test_pipeline.py b/tests/test_pipeline.py index 01ea617..3200104 100644 --- a/tests/test_pipeline.py +++ b/tests/test_pipeline.py @@ -1,3 +1,5 @@ +# This tests some internals +# ruff: noqa:SLF001 from datetime import timedelta import pytest @@ -199,7 +201,12 @@ def test_graph_computation_is_correct( pipeline.register_step("step-1-3-1", None, func()) # pylint: disable=protected-access - assert pipeline._build_graph(steps, except_steps, only_selected) == result + assert ( + pipeline._build_graph( + steps, except_steps, only_selected_steps=only_selected + ) + == result + ) def test_only_keeps_dependency_information(pipeline): @@ -209,10 +216,9 @@ def test_only_keeps_dependency_information(pipeline): pipeline.register_step("4", None, func()) # pylint: disable=protected-access - assert pipeline._build_graph(["1", "4"], None, True) == { - "1": ["4"], - "4": [], - } + assert pipeline._build_graph( + ["1", "4"], None, only_selected_steps=True + ) == {"1": ["4"], "4": []} def test_exclude_keeps_dependency_information(pipeline): @@ -222,13 +228,12 @@ def test_exclude_keeps_dependency_information(pipeline): pipeline.register_step("4", None, func()) # pylint: disable=protected-access - assert pipeline._build_graph(None, ["2", "3"], False) == { - "1": ["4"], - "4": [], - } + assert pipeline._build_graph( + None, ["2", "3"], only_selected_steps=False + ) == {"1": ["4"], "4": []} -@pytest.mark.parametrize("step_type", ["normal", "group"]) +@pytest.mark.parametrize("step_type", ("normal", "group")) def test_cannot_register_two_step_with_same_name(pipeline, step_type): pipeline.register_step("step", None, func()) if step_type == "group": diff --git a/tests/test_registration.py b/tests/test_registration.py index b83920c..b296cf4 100644 --- a/tests/test_registration.py +++ b/tests/test_registration.py @@ -1,3 +1,5 @@ +# This tests some internals +# ruff: noqa:SLF001 from typing import Any, Dict, Optional import pytest @@ -132,7 +134,7 @@ def __call__(self) -> None: @pytest.mark.parametrize( - "from_parameters", [True, False], ids=["from_parameters", "direct"] + "from_parameters", (True, False), ids=["from_parameters", "direct"] ) @isolated_context def test_can_register_managed_step(pipeline, from_parameters):