diff --git a/README.md b/README.md index 887655b..9445513 100644 --- a/README.md +++ b/README.md @@ -5,6 +5,29 @@ Simple python based helpers for lobaro tools. This package does not have any dependencies and should easily integrate into other packages. +## Renaming a command line option + +`add_renamed_argument` adds an option and keeps earlier spellings of it working, +so a rename does not break anyone's scripts: + +```python +import argparse +from lob_hlpr import add_renamed_argument + +parser = argparse.ArgumentParser() +add_renamed_argument(parser, "--log-level", deprecated="--loglevel", default="WARNING") + +parser.parse_args(["--loglevel", "DEBUG"]).log_level # "DEBUG", with a warning +``` + +The old spelling writes to the same destination as the current one, so the rest +of the program never learns it exists and no fixup step is needed after parsing. +It is hidden from `--help` and warns when used, both as a `DeprecationWarning` +and as a log record, so a user sees it even before logging is configured. + +Pass a list to `deprecated` for an option that has been renamed more than once. +Options that take no value, such as `action="store_true"`, work too. + ## Installation This package should be available in `pypi` and can be installed with `pip` or diff --git a/src/lob_hlpr/__init__.py b/src/lob_hlpr/__init__.py index 730e565..d8ec8af 100644 --- a/src/lob_hlpr/__init__.py +++ b/src/lob_hlpr/__init__.py @@ -3,6 +3,7 @@ Simple python based helpers for lobaro tools. """ +from lob_hlpr.cli import DeprecatedAliasAction, add_renamed_argument from lob_hlpr.hlpr import LobHlpr from lob_hlpr.lib_types import FirmwareID, FirmwareVersion @@ -10,4 +11,6 @@ "LobHlpr", "FirmwareID", "FirmwareVersion", + "add_renamed_argument", + "DeprecatedAliasAction", ] diff --git a/src/lob_hlpr/cli.py b/src/lob_hlpr/cli.py new file mode 100644 index 0000000..ec00461 --- /dev/null +++ b/src/lob_hlpr/cli.py @@ -0,0 +1,93 @@ +"""Helpers for building the command line interfaces of the Lobaro tools.""" + +import argparse +import logging +import warnings +from collections.abc import Sequence +from typing import Any + +_LOGGER = logging.getLogger(__name__) + + +class DeprecatedAliasAction(argparse.Action): + """Store a value like the option it replaces, but say it is out of date. + + Rarely used directly, :func:`add_renamed_argument` wires it up. + """ + + def __init__(self, option_strings: Sequence[str], dest: str, **kwargs: Any): + """Take the replacement flag out of the keywords argparse does not know.""" + self.use_instead: str = kwargs.pop("use_instead", dest) + super().__init__(option_strings, dest, **kwargs) + + def __call__(self, parser, namespace, values, option_string=None): + """Warn about the old spelling, then store as the current one would.""" + message = f"{option_string} is deprecated, use {self.use_instead} instead." + warnings.warn(message, DeprecationWarning, stacklevel=2) + # Logging rather than print, and visible without a configured handler + # because logging falls back to writing warnings to stderr. + _LOGGER.warning(message) + # nargs == 0 means a flag such as store_true, which carries its value in + # const instead of on the command line. + setattr(namespace, self.dest, self.const if self.nargs == 0 else values) + + +def add_renamed_argument( + parser: argparse.ArgumentParser, + *flags: str, + deprecated: str | Sequence[str], + **kwargs: Any, +) -> argparse.Action: + """Add an option and keep earlier spellings of it working. + + The old spellings write to the same destination as the current one, so + nothing downstream has to know they exist. They are hidden from ``--help`` + and warn when used, both as a :class:`DeprecationWarning` and as a log + record, so a user sees the message even before logging is configured. + + Args: + parser: The parser to extend. + flags: The current flags, for example ``"--log-level"``. + deprecated: Old flag or flags that should still work, for example + ``"--loglevel"``. + kwargs: Passed through to :meth:`argparse.ArgumentParser.add_argument`. + + Returns: + The action for the current option. + + Example: + >>> parser = argparse.ArgumentParser() + >>> _ = add_renamed_argument( + ... parser, "--log-level", deprecated="--loglevel", default="WARNING" + ... ) + >>> parser.parse_args(["--loglevel", "DEBUG"]).log_level + 'DEBUG' + """ + action = parser.add_argument(*flags, **kwargs) + # argparse's own default for these is not None, so only forward the ones the + # current option actually set. + inherited: dict[str, Any] = { + name: value + for name, value in ( + ("nargs", action.nargs), + ("const", action.const), + ("type", action.type), + ("choices", action.choices), + ) + if value is not None + } + old_flags = [deprecated] if isinstance(deprecated, str) else list(deprecated) + for old_flag in old_flags: + parser.add_argument( + old_flag, + action=DeprecatedAliasAction, + use_instead=action.option_strings[0], + dest=action.dest, + # Never supply a default, that is the current option's job. Without + # this the alias would overwrite it with None. + default=argparse.SUPPRESS, + required=False, + help=argparse.SUPPRESS, + **inherited, + ) + return action diff --git a/tests/test_cli.py b/tests/test_cli.py new file mode 100644 index 0000000..56668cb --- /dev/null +++ b/tests/test_cli.py @@ -0,0 +1,119 @@ +"""Tests for renaming a command line option without breaking the old spelling.""" + +import argparse +import logging + +import pytest + +from lob_hlpr import add_renamed_argument + + +@pytest.fixture +def parser(): + """A parser with a renamed option that takes a value.""" + parser = argparse.ArgumentParser("test", exit_on_error=False) + add_renamed_argument( + parser, "--log-level", deprecated="--loglevel", default="WARNING" + ) + return parser + + +def test_current_spelling_is_stored(parser): + """The new flag behaves like any other option.""" + assert parser.parse_args(["--log-level", "DEBUG"]).log_level == "DEBUG" + + +def test_deprecated_spelling_lands_in_the_same_place(parser): + """Callers read one attribute and never learn which spelling was used.""" + with pytest.deprecated_call(): + args = parser.parse_args(["--loglevel", "DEBUG"]) + assert args.log_level == "DEBUG" + + +def test_default_survives_the_alias(parser): + """The alias must not overwrite the default with an empty value.""" + assert parser.parse_args([]).log_level == "WARNING" + + +def test_current_spelling_does_not_warn(parser, recwarn): + """Only the old spelling is worth complaining about.""" + parser.parse_args(["--log-level", "DEBUG"]) + assert not recwarn.list + + +def test_the_last_flag_given_wins(parser): + """Mixing spellings is odd but should not need a precedence rule.""" + with pytest.deprecated_call(): + args = parser.parse_args(["--loglevel", "DEBUG", "--log-level", "INFO"]) + assert args.log_level == "INFO" + + +def test_deprecated_spelling_is_hidden_from_help(parser): + """The old flag still works but should not be advertised.""" + help_text = parser.format_help() + assert "--log-level" in help_text + assert "--loglevel" not in help_text + + +def test_the_warning_names_both_spellings(parser): + """A warning the user cannot act on is not worth printing.""" + with pytest.warns(DeprecationWarning) as warnings_raised: + parser.parse_args(["--loglevel", "DEBUG"]) + message = str(warnings_raised[0].message) + assert "--loglevel" in message + assert "--log-level" in message + + +def test_the_warning_is_logged_too(parser, caplog): + """A DeprecationWarning alone is hidden by default, a user sees nothing.""" + with caplog.at_level(logging.WARNING), pytest.deprecated_call(): + parser.parse_args(["--loglevel", "DEBUG"]) + assert "--loglevel is deprecated" in caplog.text + + +def test_the_value_is_converted_like_the_current_option(): + """The alias has to apply the same type, not hand back a string.""" + parser = argparse.ArgumentParser("test") + add_renamed_argument(parser, "--retry-count", deprecated="--retrycount", type=int) + with pytest.deprecated_call(): + assert parser.parse_args(["--retrycount", "3"]).retry_count == 3 + + +def test_a_flag_without_a_value_can_be_renamed(): + """store_true carries its value in const rather than on the command line.""" + parser = argparse.ArgumentParser("test") + add_renamed_argument(parser, "--no-gui", deprecated="--nogui", action="store_true") + assert parser.parse_args([]).no_gui is False + with pytest.deprecated_call(): + assert parser.parse_args(["--nogui"]).no_gui is True + + +def test_several_old_spellings_can_be_kept_alive(): + """An option renamed twice should not need two helpers.""" + parser = argparse.ArgumentParser("test") + add_renamed_argument( + parser, "--log-level", deprecated=["--loglevel", "--log_level"] + ) + with pytest.deprecated_call(): + assert parser.parse_args(["--log_level", "DEBUG"]).log_level == "DEBUG" + with pytest.deprecated_call(): + assert parser.parse_args(["--loglevel", "INFO"]).log_level == "INFO" + + +def test_a_short_flag_can_be_added_alongside(): + """Tools differ on whether the short flag is free to use.""" + parser = argparse.ArgumentParser("test") + add_renamed_argument(parser, "--one-cmd", "-o", deprecated="--onecmd") + assert parser.parse_args(["-o", "info"]).one_cmd == "info" + with pytest.deprecated_call(): + assert parser.parse_args(["--onecmd", "info"]).one_cmd == "info" + + +def test_choices_are_enforced_for_the_old_spelling_too(): + """An alias that skips validation would be a hole in the interface.""" + parser = argparse.ArgumentParser("test", exit_on_error=False) + add_renamed_argument( + parser, "--mode", deprecated="--Mode", choices=["fast", "slow"] + ) + with pytest.raises(argparse.ArgumentError): + parser.parse_args(["--Mode", "sideways"])