Skip to content

Commit

Permalink
refactor!: Switch some args to keyword-only
Browse files Browse the repository at this point in the history
Ruff discovered a number of instances in which boolean arguments were
being used in functions:

* https://docs.astral.sh/ruff/rules/boolean-type-hint-positional-argument/
* https://docs.astral.sh/ruff/rules/boolean-default-value-positional-argument/

The intent all along was that these (along with any others that had
default values associated) be keyword-only arguments, but they were not
denoted as such.  This commit remedies that oversight.

Note that this is a breaking change because it changes the call
signatures of the `Shell` and `ShellLogger` public classes.  To update
your usage, simply switch from positional to keyword arguments for any
arguments affected.
  • Loading branch information
jmgate committed Apr 24, 2024
1 parent a81af8a commit af677a1
Show file tree
Hide file tree
Showing 9 changed files with 61 additions and 32 deletions.
4 changes: 2 additions & 2 deletions doc/source/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ highlighted lines below.
:language: python
:linenos:
:lines: 10-
:emphasize-lines: 15-16, 22-23
:emphasize-lines: 16-17, 23-24
:caption: ``example/hello_world_html_and_console.py``

Example 3: Collecting Statistics
Expand All @@ -251,7 +251,7 @@ and **Example 1** are the highlighted lines below.
:language: python
:linenos:
:lines: 10-
:emphasize-lines: 16, 22
:emphasize-lines: 17, 23
:caption: ``example/hello_world_html_with_stats.py``

Example 4: Building a Code
Expand Down
5 changes: 4 additions & 1 deletion example/build_flex.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@

from shell_logger import ShellLogger

sl = ShellLogger("Build Flex", Path.cwd() / f"log_{Path(__file__).stem}")
sl = ShellLogger(
"Build Flex",
log_dir=(Path.cwd() / f"log_{Path(__file__).stem}"),
)
sl.print(
"This example demonstrates cloning, configuring, and building the Flex "
"tool."
Expand Down
5 changes: 4 additions & 1 deletion example/hello_world_html.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@

from shell_logger import ShellLogger

sl = ShellLogger("Hello World HTML", Path.cwd() / f"log_{Path(__file__).stem}")
sl = ShellLogger(
"Hello World HTML",
log_dir=(Path.cwd() / f"log_{Path(__file__).stem}"),
)
sl.print(
"This example demonstrates logging information solely to the HTML log "
"file."
Expand Down
3 changes: 2 additions & 1 deletion example/hello_world_html_and_console.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
from shell_logger import ShellLogger

sl = ShellLogger(
"Hello World HTML and Console", Path.cwd() / f"log_{Path(__file__).stem}"
"Hello World HTML and Console",
log_dir=(Path.cwd() / f"log_{Path(__file__).stem}"),
)
sl.print(
"This example demonstrates logging information both to the HTML log file "
Expand Down
3 changes: 2 additions & 1 deletion example/hello_world_html_with_stats.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
from shell_logger import ShellLogger

sl = ShellLogger(
"Hello World HTML with Stats", Path.cwd() / f"log_{Path(__file__).stem}"
"Hello World HTML with Stats",
log_dir=(Path.cwd() / f"log_{Path(__file__).stem}"),
)
sl.print(
"This example demonstrates logging information solely to the HTML log "
Expand Down
8 changes: 6 additions & 2 deletions shell_logger/html_utilities.py
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ def command_detail_list(cmd_id: str, *args: Iterator[str]) -> Iterator[str]:


def command_detail(
cmd_id: str, name: str, value: str, hidden: bool = False
cmd_id: str, name: str, value: str, *, hidden: bool = False
) -> str:
"""
Generate the HTML for a command detail.
Expand Down Expand Up @@ -552,7 +552,11 @@ def stat_chart_card(


def output_block_card(
title: str, output: Union[Path, str], cmd_id: str, collapsed: bool = True
title: str,
output: Union[Path, str],
cmd_id: str,
*,
collapsed: bool = True,
) -> Iterator[str]:
"""
Generate an output block card.
Expand Down
2 changes: 1 addition & 1 deletion shell_logger/shell.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class Shell:
"""

def __init__(
self, pwd: Optional[Path] = None, login_shell: bool = False
self, pwd: Optional[Path] = None, *, login_shell: bool = False
) -> None:
"""
Initialize a :class:`Shell` object.
Expand Down
34 changes: 18 additions & 16 deletions shell_logger/shell_logger.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ def append(path: Path) -> ShellLogger:
def __init__(
self,
name: str,
*,
log_dir: Optional[Path] = None,
stream_dir: Optional[Path] = None,
html_file: Optional[Path] = None,
Expand Down Expand Up @@ -187,7 +188,7 @@ def __init__(
self.duration = duration
self.indent = indent
self.login_shell = login_shell
self.shell = Shell(Path.cwd(), self.login_shell)
self.shell = Shell(Path.cwd(), login_shell=self.login_shell)

# Create the log directory, if needed.
if log_dir is None:
Expand Down Expand Up @@ -331,11 +332,11 @@ def add_child(self, child_name: str) -> ShellLogger:
# Create the child object and add it to the list of children.
child = ShellLogger(
child_name,
self.log_dir,
self.stream_dir,
self.html_file,
self.indent + 1,
self.login_shell,
log_dir=self.log_dir,
stream_dir=self.stream_dir,
html_file=self.html_file,
indent=(self.indent + 1),
login_shell=self.login_shell,
)
self.log_book.append(child)
return child
Expand Down Expand Up @@ -487,6 +488,7 @@ def log(
self,
msg: str,
cmd: str,
*,
cwd: Optional[Path] = None,
live_stdout: bool = False,
live_stderr: bool = False,
Expand Down Expand Up @@ -805,15 +807,15 @@ def dict_to_object(obj: dict) -> object:
elif obj["__type__"] == "ShellLogger":
logger = ShellLogger(
obj["name"],
obj["log_dir"],
obj["stream_dir"],
obj["html_file"],
obj["indent"],
obj["login_shell"],
obj["log_book"],
obj["init_time"],
obj["done_time"],
obj["duration"],
log_dir=obj["log_dir"],
stream_dir=obj["stream_dir"],
html_file=obj["html_file"],
indent=obj["indent"],
login_shell=obj["login_shell"],
log=obj["log_book"],
init_time=obj["init_time"],
done_time=obj["done_time"],
duration=obj["duration"],
)
return logger
elif obj["__type__"] == "datetime":
Expand All @@ -823,6 +825,6 @@ def dict_to_object(obj: dict) -> object:
elif obj["__type__"] == "tuple":
return tuple(obj["items"])
elif obj["__type__"] == "Shell":
return Shell(Path(obj["pwd"]), obj["login_shell"])
return Shell(Path(obj["pwd"]), login_shell=obj["login_shell"])
else:
return None
29 changes: 22 additions & 7 deletions test/test_shell_logger.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ def shell_logger() -> ShellLogger:
print(
f"Warning: uname is not 'Linux': {os.uname()}; ltrace not tested."
)
parent.log("test cmd", cmd, Path.cwd(), **kwargs)
parent.log("test cmd", cmd, cwd=Path.cwd(), **kwargs)
parent.print("This is a message")

# Add a child and run some commands.
Expand Down Expand Up @@ -143,7 +143,9 @@ def test_log_method_creates_tmp_stdout_stderr_files(


@pytest.mark.parametrize("return_info", [True, False])
def test_log_method_return_info_works_correctly(return_info: bool) -> None:
def test_log_method_return_info_works_correctly(
return_info: bool, # noqa: FBT001
) -> None:
"""
Ensure ``stdout``/``stderr`` are returned when requested.
Expand All @@ -160,7 +162,12 @@ def test_log_method_return_info_works_correctly(return_info: bool) -> None:

# `stdout` ; `stderr`
cmd = "echo 'Hello world out'; echo 'Hello world error' 1>&2"
result = logger.log("test cmd", cmd, Path.cwd(), return_info=return_info)
result = logger.log(
"test cmd",
cmd,
cwd=Path.cwd(),
return_info=return_info,
)
if return_info:
assert "Hello world out" in result["stdout"]
assert "Hello world error" in result["stderr"]
Expand All @@ -174,7 +181,9 @@ def test_log_method_return_info_works_correctly(return_info: bool) -> None:
@pytest.mark.parametrize("live_stdout", [True, False])
@pytest.mark.parametrize("live_stderr", [True, False])
def test_log_method_live_stdout_stderr_works_correctly(
capsys: CaptureFixture, live_stdout: bool, live_stderr: bool
capsys: CaptureFixture,
live_stdout: bool, # noqa: FBT001
live_stderr: bool, # noqa: FBT001
) -> None:
"""
Ensure live streaming of ``stdout``/``stderr`` works.
Expand All @@ -191,7 +200,13 @@ def test_log_method_live_stdout_stderr_works_correctly(
"""
logger = ShellLogger(stack()[0][3], Path.cwd())
cmd = "echo 'Hello world out'; echo 'Hello world error' 1>&2"
logger.log("test cmd", cmd, Path.cwd(), live_stdout, live_stderr)
logger.log(
"test cmd",
cmd,
cwd=Path.cwd(),
live_stdout=live_stdout,
live_stderr=live_stderr,
)
out, err = capsys.readouterr()
if live_stdout:
assert re.search(r"^Hello world out(\r)?\n", out) is not None
Expand Down Expand Up @@ -451,12 +466,12 @@ def test_logger_does_not_store_stdout_string_by_default() -> None:
def test_logger_does_not_store_trace_string_by_default() -> None:
"""Ensure we don't keep trace output in memory by default."""
logger = ShellLogger(stack()[0][3], Path.cwd())
logger.log("echo hello", "echo hello", Path.cwd(), trace="ltrace")
logger.log("echo hello", "echo hello", cwd=Path.cwd(), trace="ltrace")
assert logger.log_book[0]["trace"] is None
logger.log(
"echo hello",
"echo hello",
Path.cwd(),
cwd=Path.cwd(),
return_info=True,
trace="ltrace",
)
Expand Down

0 comments on commit af677a1

Please sign in to comment.