Skip to content

Commit

Permalink
lint(ruff): require boolean arguments to be passed as keywords
Browse files Browse the repository at this point in the history
Otherwise it can't be quite confusing and ordering becomes less clear.
Overall this reduces errors
  • Loading branch information
BenjaminSchubert committed Dec 30, 2023
1 parent 85ca4d2 commit a8335bd
Show file tree
Hide file tree
Showing 12 changed files with 63 additions and 33 deletions.
2 changes: 1 addition & 1 deletion dwasfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,5 +227,5 @@
"ci",
["docs", "format-check", "lint", "coverage", "twine:check"],
"Run all checks that are run on CI",
False,
run_by_default=False,
)
36 changes: 24 additions & 12 deletions src/dwas/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -275,8 +275,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 +297,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 +327,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,11 +346,11 @@ 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(
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:

Check warning on line 128 in src/dwas/_frontend.py

View check run for this annotation

Codecov / codecov/patch

src/dwas/_frontend.py#L128

Added line #L128 was not covered by tests
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)

Check warning on line 186 in src/dwas/_frontend.py

View check run for this annotation

Codecov / codecov/patch

src/dwas/_frontend.py#L186

Added line #L186 was not covered by tests
6 changes: 4 additions & 2 deletions src/dwas/_io.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
3 changes: 2 additions & 1 deletion src/dwas/_logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,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"
Expand Down
11 changes: 9 additions & 2 deletions src/dwas/_pipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ def _build_graph(
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
Expand Down Expand Up @@ -287,13 +288,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()
Expand Down Expand Up @@ -343,6 +347,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:
Expand All @@ -351,7 +356,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 = []
Expand Down
7 changes: 5 additions & 2 deletions src/dwas/_runners.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,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,
Expand All @@ -151,7 +154,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"])

Expand Down
2 changes: 1 addition & 1 deletion src/dwas/predefined/_package.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
1 change: 1 addition & 0 deletions src/dwas/predefined/_sphinx.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion tests/predefined/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,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")
Expand Down
21 changes: 12 additions & 9 deletions tests/test_pipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -201,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):
Expand All @@ -211,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):
Expand All @@ -224,10 +228,9 @@ 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"))
Expand Down

0 comments on commit a8335bd

Please sign in to comment.