From edad4b07685be3a238203b8cc56e01a7a181e191 Mon Sep 17 00:00:00 2001 From: Nick Murphy Date: Fri, 23 Oct 2026 17:42:37 -0400 Subject: [PATCH 1/2] Make boolean arguments keyword-only Positional boolean arguments make call sites hard to read (e.g., read_file(path, True, False)). Requiring them to be passed by name lets us drop the FBT001/FBT002 suppressions, and a new test keeps the decommutator from regressing. In swp_spc_l0_to_l05.main and _get_layout_sc, the arguments apidreq and filename follow boolean arguments, so they are now keyword-only as well. Co-Authored-By: Claude Opus 5.5 --- .../decommutator/ccsds_reader_pipeline.py | 28 ++++++++----- .../decommutator/swp_spc_l0_to_l05.py | 40 +++++++++++-------- tests/decommutator/test_signatures.py | 40 +++++++++++++++++++ 3 files changed, 80 insertions(+), 28 deletions(-) create mode 100644 tests/decommutator/test_signatures.py diff --git a/src/pyfaradaycup/decommutator/ccsds_reader_pipeline.py b/src/pyfaradaycup/decommutator/ccsds_reader_pipeline.py index bf63dfd..bebf3f6 100644 --- a/src/pyfaradaycup/decommutator/ccsds_reader_pipeline.py +++ b/src/pyfaradaycup/decommutator/ccsds_reader_pipeline.py @@ -33,7 +33,8 @@ def _file2bytestr( path: str | os.PathLike[str], - gzip: bool = False, # ruff:ignore[FBT001, FBT002] + *, + gzip: bool = False, ) -> bytes: """ Read the entire contents of a file into a bytes object. @@ -104,8 +105,9 @@ def _choose_file(path: str | os.PathLike[str]) -> str: def _wrapper_status( path: str | os.PathLike[str], - gzip: bool = False, # ruff:ignore[FBT001, FBT002] - spconly: bool = False, # ruff:ignore[FBT001, FBT002] + *, + gzip: bool = False, + spconly: bool = False, ) -> dict[str, list[int]]: """ Read CCSDS headers from SWEM wrapper packets and the packets inside them. @@ -198,8 +200,9 @@ def _wrapper_status( def read_file( # ruff:ignore[C901] path: str | os.PathLike[str], - verbose: bool = False, # ruff:ignore[FBT001, FBT002] - gzip: bool = False, # ruff:ignore[FBT001, FBT002] + *, + verbose: bool = False, + gzip: bool = False, ) -> dict[int, dict[str, list]]: """ Read SPC instrument packets from an L0 file. @@ -326,9 +329,10 @@ def read_file( # ruff:ignore[C901] def read_file_sc( # ruff:ignore[C901, PLR0912, PLR0915] path: str | os.PathLike[str], - verbose: bool = False, # ruff:ignore[FBT001, FBT002] - ptp: bool = False, # ruff:ignore[FBT001, FBT002] - gzip: bool = False, # ruff:ignore[FBT001, FBT002] + *, + verbose: bool = False, + ptp: bool = False, + gzip: bool = False, ) -> dict[int, dict[str, list]]: """ Read spacecraft housekeeping packets from an L0 file. @@ -575,7 +579,8 @@ def _read_bytestr( # ruff:ignore[C901, PLR0912, PLR0913] data: dict[int, dict[str, list]], apidformat, # ruff:ignore[ANN001] pktcnt: list[dict[int, int]], - verbose: bool = False, # ruff:ignore[FBT001, FBT002] + *, + verbose: bool = False, ) -> tuple[()]: """Take a hex string and find packets""" # ruff:ignore[D400] # Parse the CCSDS header @@ -853,7 +858,7 @@ def __init__(self) -> None: self.startbit: list[int] = [] -def _get_layout(apid: int, verbose: bool = False) -> apid_obj | None: # ruff:ignore[C901, FBT001, FBT002] +def _get_layout(apid: int, *, verbose: bool = False) -> apid_obj | None: # ruff:ignore[C901] """ Read the bit layout for one SWEAP APID from ``sweap_tlm.blk``. @@ -942,7 +947,8 @@ def _get_layout(apid: int, verbose: bool = False) -> apid_obj | None: # ruff:ig def _get_layout_sc( # ruff:ignore[C901] apid: int, - verbose: bool = False, # ruff:ignore[FBT001, FBT002] + *, + verbose: bool = False, filename: str | os.PathLike[str] | None = None, ) -> tuple[apid_obj, int] | None: """ diff --git a/src/pyfaradaycup/decommutator/swp_spc_l0_to_l05.py b/src/pyfaradaycup/decommutator/swp_spc_l0_to_l05.py index d07df0b..ed55605 100644 --- a/src/pyfaradaycup/decommutator/swp_spc_l0_to_l05.py +++ b/src/pyfaradaycup/decommutator/swp_spc_l0_to_l05.py @@ -61,16 +61,17 @@ logfile: TextIO -def main( # ruff:ignore[C901, PLR0912, PLR0913, PLR0915, PLR0917] +def main( # ruff:ignore[C901, PLR0912, PLR0913, PLR0915] l0file: str | os.PathLike[str], l05dir: str | os.PathLike[str] | None = None, logdir: str | os.PathLike[str] | None = None, - spacecraft: bool = False, # ruff:ignore[FBT001, FBT002] - ptp: bool = False, # ruff:ignore[FBT001, FBT002] - gzip: bool = False, # ruff:ignore[FBT001, FBT002] + *, + spacecraft: bool = False, + ptp: bool = False, + gzip: bool = False, apidreq: int = 0, - overwrite: bool = False, # ruff:ignore[FBT001, FBT002] - verbose: bool = False, # ruff:ignore[FBT001, FBT002] + overwrite: bool = False, + verbose: bool = False, ) -> None: """ Convert one SPC L0 file into L0.5 CDF files, one per APID. @@ -367,7 +368,7 @@ def main( # ruff:ignore[C901, PLR0912, PLR0913, PLR0915, PLR0917] logfile.close() -def _cdf35e_35f(cdf: pycdf.CDF, dat: dict[str, list], verbose: bool = False) -> None: # ruff:ignore[ARG001, C901, FBT001, FBT002] +def _cdf35e_35f(cdf: pycdf.CDF, dat: dict[str, list], *, verbose: bool = False) -> None: # ruff:ignore[ARG001, C901] """ Fill a CDF with housekeeping data, one row per packet. @@ -466,8 +467,9 @@ def _cdf35e_35f(cdf: pycdf.CDF, dat: dict[str, list], verbose: bool = False) -> def _cdf351_353_354( # ruff:ignore[C901, PLR0912, PLR0915, RET503] cdf: pycdf.CDF, dat: dict[str, list], - nocdf: bool = False, # ruff:ignore[FBT001, FBT002] - verbose: bool = False, # ruff:ignore[FBT001, FBT002] + *, + nocdf: bool = False, + verbose: bool = False, ) -> dict[str, list] | None: """ Expand SPC science packets (APIDs 0x351, 0x353, 0x354) and write them to a CDF. @@ -668,8 +670,9 @@ def _cdf351_353_354( # ruff:ignore[C901, PLR0912, PLR0915, RET503] def _cdf352( # ruff:ignore[C901, PLR0912, PLR0915] cdf: pycdf.CDF, dat: dict[str, list], - nocdf: bool = False, # ruff:ignore[FBT001, FBT002] - verbose: bool = False, # ruff:ignore[FBT001, FBT002] + *, + nocdf: bool = False, + verbose: bool = False, ) -> dict[str, list] | tuple[()]: """ Expand SPC time series (APID 0x352) packets into L0.5 data and write them to a CDF. @@ -866,7 +869,8 @@ def _cdf352( # ruff:ignore[C901, PLR0912, PLR0915] def _secsubsec2scet( sec: list[int], subsec: list[int], - spacecraft: bool = False, # ruff:ignore[FBT001, FBT002] + *, + spacecraft: bool = False, ) -> list[float]: """ Convert MET seconds and subseconds to ephemeris time in nanoseconds. @@ -920,9 +924,10 @@ def _secsubsec2scet( def _statusmsg( string: str, - screen: bool = False, # ruff:ignore[FBT001, FBT002] - file: bool = True, # ruff:ignore[FBT001, FBT002] - verbose: bool = False, # ruff:ignore[FBT001, FBT002] + *, + screen: bool = False, + file: bool = True, + verbose: bool = False, ) -> None: """ Write a timestamped status message to the log file and/or the screen. @@ -958,8 +963,9 @@ def _statusmsg( def _get_newest_kernel( - tls: bool = False, # ruff:ignore[FBT001, FBT002] - sclk: bool = False, # ruff:ignore[FBT001, FBT002] + *, + tls: bool = False, + sclk: bool = False, ) -> str: """ Find the newest NAIF leap second or PSP clock (SCLK) kernel file. diff --git a/tests/decommutator/test_signatures.py b/tests/decommutator/test_signatures.py new file mode 100644 index 0000000..6466182 --- /dev/null +++ b/tests/decommutator/test_signatures.py @@ -0,0 +1,40 @@ +"""Test that boolean parameters in the decommutator are keyword-only.""" + +import inspect +from collections.abc import Callable +from types import ModuleType + +import pytest + +from pyfaradaycup.decommutator import ( + ccsds_reader_pipeline, + main, + swp_spc_l0_to_l05, +) + + +def _functions(module: ModuleType) -> list[Callable]: + return [ + obj + for _, obj in inspect.getmembers(module, inspect.isfunction) + if obj.__module__ == module.__name__ + ] + + +functions = [ + func + for module in (ccsds_reader_pipeline, main, swp_spc_l0_to_l05) + for func in _functions(module) +] + + +@pytest.mark.parametrize("func", functions, ids=lambda func: func.__qualname__) +def test_bool_parameters_are_keyword_only(func: Callable) -> None: + """Test that parameters annotated as `bool` are keyword-only.""" + signature = inspect.signature(func) + positional_bools = [ + name + for name, param in signature.parameters.items() + if param.annotation in {bool, "bool"} and param.kind is not param.KEYWORD_ONLY + ] + assert not positional_bools From 85fa197abb4d61fce59426b50cac50bf6fc637d7 Mon Sep 17 00:00:00 2001 From: Nick Murphy Date: Fri, 23 Oct 2026 17:48:48 -0400 Subject: [PATCH 2/2] Remove test of keyword-only boolean arguments Co-Authored-By: Claude Opus 5.5 --- tests/decommutator/test_signatures.py | 40 --------------------------- 1 file changed, 40 deletions(-) delete mode 100644 tests/decommutator/test_signatures.py diff --git a/tests/decommutator/test_signatures.py b/tests/decommutator/test_signatures.py deleted file mode 100644 index 6466182..0000000 --- a/tests/decommutator/test_signatures.py +++ /dev/null @@ -1,40 +0,0 @@ -"""Test that boolean parameters in the decommutator are keyword-only.""" - -import inspect -from collections.abc import Callable -from types import ModuleType - -import pytest - -from pyfaradaycup.decommutator import ( - ccsds_reader_pipeline, - main, - swp_spc_l0_to_l05, -) - - -def _functions(module: ModuleType) -> list[Callable]: - return [ - obj - for _, obj in inspect.getmembers(module, inspect.isfunction) - if obj.__module__ == module.__name__ - ] - - -functions = [ - func - for module in (ccsds_reader_pipeline, main, swp_spc_l0_to_l05) - for func in _functions(module) -] - - -@pytest.mark.parametrize("func", functions, ids=lambda func: func.__qualname__) -def test_bool_parameters_are_keyword_only(func: Callable) -> None: - """Test that parameters annotated as `bool` are keyword-only.""" - signature = inspect.signature(func) - positional_bools = [ - name - for name, param in signature.parameters.items() - if param.annotation in {bool, "bool"} and param.kind is not param.KEYWORD_ONLY - ] - assert not positional_bools