Skip to content

Commit

Permalink
Configure ruff for this project and fix problems
Browse files Browse the repository at this point in the history
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
  • Loading branch information
BenjaminSchubert committed Dec 30, 2023
1 parent 8d60c5d commit 638611f
Show file tree
Hide file tree
Showing 28 changed files with 277 additions and 127 deletions.
9 changes: 6 additions & 3 deletions .github/scripts/summary.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# ruff: noqa:D100,D101,D102,D103,D105
import argparse
import logging
from dataclasses import InitVar, dataclass, field
Expand Down Expand Up @@ -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", "<br />")

Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -168,7 +171,7 @@ def main() -> None:

errors = get_failures_and_errors(testsuites)

print(
print( # noqa: T201
f"""\
## Test suites
Expand Down
1 change: 1 addition & 0 deletions docs/_extensions/cleanup_signatures.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from sphinx.ext.autodoc import Options


# ruff: noqa: ARG001
def cleanup_signatures( # pylint: disable=unused-argument
app: Sphinx,
what: str,
Expand Down
10 changes: 7 additions & 3 deletions docs/_extensions/execute.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand All @@ -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
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions docs/_extensions/styles.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
2 changes: 1 addition & 1 deletion docs/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
5 changes: 4 additions & 1 deletion dwasfile.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
"""
Contains the dwas configuration for this project.
"""
from pathlib import Path

import dwas
Expand Down Expand Up @@ -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,
)
72 changes: 72 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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",
]


##
Expand Down
47 changes: 30 additions & 17 deletions src/dwas/__main__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# ruff: noqa:D100,D101,D102,D103
import importlib.util
import logging
import os
Expand Down Expand Up @@ -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:
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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()
Expand All @@ -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"
Expand All @@ -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)
Expand Down
1 change: 1 addition & 0 deletions src/dwas/_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ def __init__(
verbosity: int,
colors: Optional[bool],
n_jobs: int,
*,
skip_missing_interpreters: bool,
skip_setup: bool,
skip_run: bool,
Expand Down
4 changes: 2 additions & 2 deletions src/dwas/_frontend.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -183,4 +183,4 @@ def refresh(skip_summary: bool = False) -> None:
self._stop.wait(0.5)
refresh()

refresh(True)
refresh(skip_summary=True)
2 changes: 1 addition & 1 deletion src/dwas/_inspect.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
12 changes: 7 additions & 5 deletions src/dwas/_io.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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:
Expand All @@ -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)

Expand All @@ -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:
Expand Down Expand Up @@ -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:
Expand Down
Loading

0 comments on commit 638611f

Please sign in to comment.