From af677a1156715c579f8b2c34dca105921093fb6d Mon Sep 17 00:00:00 2001 From: "Jason M. Gates" Date: Wed, 24 Apr 2024 13:30:24 -0600 Subject: [PATCH] refactor!: Switch some args to keyword-only 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. --- doc/source/index.rst | 4 +-- example/build_flex.py | 5 +++- example/hello_world_html.py | 5 +++- example/hello_world_html_and_console.py | 3 ++- example/hello_world_html_with_stats.py | 3 ++- shell_logger/html_utilities.py | 8 ++++-- shell_logger/shell.py | 2 +- shell_logger/shell_logger.py | 34 +++++++++++++------------ test/test_shell_logger.py | 29 ++++++++++++++++----- 9 files changed, 61 insertions(+), 32 deletions(-) diff --git a/doc/source/index.rst b/doc/source/index.rst index 5036412..e464c61 100644 --- a/doc/source/index.rst +++ b/doc/source/index.rst @@ -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 @@ -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 diff --git a/example/build_flex.py b/example/build_flex.py index 057c272..6f146fd 100755 --- a/example/build_flex.py +++ b/example/build_flex.py @@ -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." diff --git a/example/hello_world_html.py b/example/hello_world_html.py index d6fa252..4436726 100755 --- a/example/hello_world_html.py +++ b/example/hello_world_html.py @@ -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." diff --git a/example/hello_world_html_and_console.py b/example/hello_world_html_and_console.py index efdbcd4..37ed8ac 100755 --- a/example/hello_world_html_and_console.py +++ b/example/hello_world_html_and_console.py @@ -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 " diff --git a/example/hello_world_html_with_stats.py b/example/hello_world_html_with_stats.py index bbd3cf2..06e3d50 100755 --- a/example/hello_world_html_with_stats.py +++ b/example/hello_world_html_with_stats.py @@ -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 " diff --git a/shell_logger/html_utilities.py b/shell_logger/html_utilities.py index 588906f..5dddba2 100644 --- a/shell_logger/html_utilities.py +++ b/shell_logger/html_utilities.py @@ -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. @@ -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. diff --git a/shell_logger/shell.py b/shell_logger/shell.py index a105132..8c2626a 100644 --- a/shell_logger/shell.py +++ b/shell_logger/shell.py @@ -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. diff --git a/shell_logger/shell_logger.py b/shell_logger/shell_logger.py index 2fceb60..e2aabb1 100644 --- a/shell_logger/shell_logger.py +++ b/shell_logger/shell_logger.py @@ -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, @@ -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: @@ -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 @@ -487,6 +488,7 @@ def log( self, msg: str, cmd: str, + *, cwd: Optional[Path] = None, live_stdout: bool = False, live_stderr: bool = False, @@ -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": @@ -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 diff --git a/test/test_shell_logger.py b/test/test_shell_logger.py index ecb6603..85aadec 100644 --- a/test/test_shell_logger.py +++ b/test/test_shell_logger.py @@ -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. @@ -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. @@ -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"] @@ -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. @@ -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 @@ -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", )