diff --git a/CHANGES.rst b/CHANGES.rst index 426a56d..79cc368 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -2,6 +2,33 @@ Changelog ~~~~~~~~~ +Version 0.30.4 (2023-11-04) +--------------------------- + +There were complaints about the lack of a deprecation cycle for the legacy name +mapping policy. This version addresses the issue: + +- The handling introduced in v.0.30.2 (raising an exception for clarity) + is retained for cases when no name mapping policy is specified but function + signature contains defaults in non-kwonly args **and kwonly args are also + defined**:: + + def main(alpha, beta=1, *, gamma=2): # error — explicit policy required + + In a similar case but when **kwonly args are not defined** Argh now assumes + the legacy name mapping policy (`BY_NAME_IF_HAS_DEFAULT`) and merely issues + a deprecation warning with the same message as the exception mentioned above:: + + def main(alpha, beta=2): # `[-b BETA] alpha` + DeprecationWarning + + This ensures that most of the old scripts still work the same way despite the + new policy being used by default and enforced in cases when it's impossible + to resolve the mapping conflict. + + Please note that this "soft" handling is to be removed in version v0.33 + (or v1.0 if the former is not deemed necessary). The new name mapping policy + will be used by default without warnings, like in v0.30. + Version 0.30.3 (2023-10-30) --------------------------- diff --git a/pyproject.toml b/pyproject.toml index 26c7cce..499cb04 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "flit_core.buildapi" [project] name = "argh" -version = "0.30.3" +version = "0.30.4" description = "An unobtrusive argparse wrapper with natural syntax" readme = "README.rst" requires-python = ">=3.8" diff --git a/src/argh/assembling.py b/src/argh/assembling.py index 381fd61..0d2786b 100644 --- a/src/argh/assembling.py +++ b/src/argh/assembling.py @@ -15,6 +15,7 @@ """ import inspect import textwrap +import warnings from argparse import OPTIONAL, ZERO_OR_MORE, ArgumentParser from collections import OrderedDict from enum import Enum @@ -115,6 +116,7 @@ def infer_argspecs_from_function( raise NotImplementedError(f"Unknown name mapping policy {name_mapping_policy}") func_spec = get_arg_spec(function) + has_kwonly = bool(func_spec.kwonlyargs) default_by_arg_name: Dict[str, Any] = dict( zip(reversed(func_spec.args), reversed(func_spec.defaults or tuple())) @@ -151,36 +153,45 @@ def _make_cli_arg_names_options(arg_name) -> Tuple[List[str], List[str]]: default_value = default_by_arg_name.get(arg_name, NotDefined) if default_value != NotDefined and not name_mapping_policy: - # TODO: remove this after something like v.0.31+2 - raise ArgumentNameMappingError( - textwrap.dedent( - f""" - Argument "{arg_name}" in function "{function.__name__}" - is not keyword-only but has a default value. + message = textwrap.dedent( + f""" + Argument "{arg_name}" in function "{function.__name__}" + is not keyword-only but has a default value. - Please note that since Argh v.0.30 the default name mapping - policy has changed. + Please note that since Argh v.0.30 the default name mapping + policy has changed. - More information: - https://argh.readthedocs.io/en/latest/changes.html#version-0-30-0-2023-10-21 + More information: + https://argh.readthedocs.io/en/latest/changes.html#version-0-30-0-2023-10-21 - You need to upgrade your functions so that the arguments - that have default values become keyword-only: + You need to upgrade your functions so that the arguments + that have default values become keyword-only: - f(x=1) -> f(*, x=1) + f(x=1) -> f(*, x=1) - If you actually want an optional positional argument, - please set the name mapping policy explicitly to `BY_NAME_IF_KWONLY`. + If you actually want an optional positional argument, + please set the name mapping policy explicitly to `BY_NAME_IF_KWONLY`. - If you choose to postpone the migration, you have two options: + If you choose to postpone the migration, you have two options: - a) set the policy explicitly to `BY_NAME_IF_HAS_DEFAULT`; - b) pin Argh version to 0.29 until you are ready to migrate. + a) set the policy explicitly to `BY_NAME_IF_HAS_DEFAULT`; + b) pin Argh version to 0.29 until you are ready to migrate. - Thank you for understanding! - """ - ).strip() - ) + Thank you for understanding! + """ + ).strip() + + # Assume legacy policy and show a warning if the signature is + # simple (without kwonly args) so that the script continues working + # but the author is urged to upgrade it. + # When it cannot be auto-resolved (kwonly args mixed with old-style + # ones but no policy specified), throw an error. + # + # TODO: remove in v.0.33 if it happens, otherwise in v1.0. + if has_kwonly: + raise ArgumentNameMappingError(message) + warnings.warn(DeprecationWarning(message)) + name_mapping_policy = NameMappingPolicy.BY_NAME_IF_HAS_DEFAULT arg_spec = ParserAddArgumentSpec( func_arg_name=arg_name, diff --git a/tests/test_mapping_policies.py b/tests/test_mapping_policies.py index a5e5204..db67e71 100644 --- a/tests/test_mapping_policies.py +++ b/tests/test_mapping_policies.py @@ -19,7 +19,7 @@ def func() -> None: ... parser = _make_parser_for_function(func, name_mapping_policy=name_mapping_policy) - assert_usage(parser, "usage: test [-h]\n") + assert_usage(parser, "usage: test [-h]") @pytest.mark.parametrize("name_mapping_policy", POLICIES) @@ -28,7 +28,7 @@ def func(alpha: str) -> str: return f"{alpha}" parser = _make_parser_for_function(func, name_mapping_policy=name_mapping_policy) - assert_usage(parser, "usage: test [-h] alpha\n") + assert_usage(parser, "usage: test [-h] alpha") assert_parsed(parser, ["hello"], Namespace(alpha="hello")) @@ -38,7 +38,7 @@ def func(alpha: str, beta: str) -> str: return f"{alpha}, {beta}" parser = _make_parser_for_function(func, name_mapping_policy=name_mapping_policy) - assert_usage(parser, "usage: test [-h] alpha beta\n") + assert_usage(parser, "usage: test [-h] alpha beta") assert_parsed(parser, ["one", "two"], Namespace(alpha="one", beta="two")) @@ -47,9 +47,9 @@ def func(alpha: str, beta: str) -> str: [ ( NameMappingPolicy.BY_NAME_IF_HAS_DEFAULT, - "usage: test [-h] [-b BETA] alpha\n", + "usage: test [-h] [-b BETA] alpha", ), - (NameMappingPolicy.BY_NAME_IF_KWONLY, "usage: test [-h] alpha [beta]\n"), + (NameMappingPolicy.BY_NAME_IF_KWONLY, "usage: test [-h] alpha [beta]"), ], ) def test_two_positionals_one_with_default(name_mapping_policy, expected_usage) -> None: @@ -74,12 +74,12 @@ def func(*file_paths) -> str: return f"{file_paths}" parser = _make_parser_for_function(func, name_mapping_policy=name_mapping_policy) - expected_usage = "usage: test [-h] [file-paths ...]\n" + expected_usage = "usage: test [-h] [file-paths ...]" # TODO: remove once we drop support for Python 3.8 if sys.version_info < (3, 9): # https://github.com/python/cpython/issues/82619 - expected_usage = "usage: test [-h] [file-paths [file-paths ...]]\n" + expected_usage = "usage: test [-h] [file-paths [file-paths ...]]" assert_usage(parser, expected_usage) @@ -87,9 +87,9 @@ def func(*file_paths) -> str: @pytest.mark.parametrize( "name_mapping_policy,expected_usage", [ - (NameMappingPolicy.BY_NAME_IF_HAS_DEFAULT, "usage: test [-h] alpha beta\n"), - (NameMappingPolicy.BY_NAME_IF_KWONLY, "usage: test [-h] -b BETA alpha\n"), - (None, "usage: test [-h] -b BETA alpha\n"), + (NameMappingPolicy.BY_NAME_IF_HAS_DEFAULT, "usage: test [-h] alpha beta"), + (NameMappingPolicy.BY_NAME_IF_KWONLY, "usage: test [-h] -b BETA alpha"), + (None, "usage: test [-h] -b BETA alpha"), ], ) def test_varargs_between_positional_and_kwonly__no_defaults( @@ -107,9 +107,9 @@ def func(alpha, *, beta) -> str: [ ( NameMappingPolicy.BY_NAME_IF_HAS_DEFAULT, - "usage: test [-h] [-a ALPHA] [-b BETA]\n", + "usage: test [-h] [-a ALPHA] [-b BETA]", ), - (NameMappingPolicy.BY_NAME_IF_KWONLY, "usage: test [-h] [-b BETA] [alpha]\n"), + (NameMappingPolicy.BY_NAME_IF_KWONLY, "usage: test [-h] [-b BETA] [alpha]"), ], ) def test_varargs_between_positional_and_kwonly__with_defaults( @@ -136,13 +136,24 @@ def func(alpha: int = 1, *, beta: int = 2) -> str: ) in str(exc.value) +# TODO: remove in v.0.33 if it happens, otherwise in v1.0. +def test_positional_with_defaults_without_kwonly__no_explicit_policy() -> None: + def func(alpha: str, beta: int = 1) -> str: + return f"{alpha} {beta}" + + message_pattern = 'Argument "beta" in function "func"\nis not keyword-only but has a default value.' + with pytest.warns(DeprecationWarning, match=message_pattern): + parser = _make_parser_for_function(func, name_mapping_policy=None) + assert_usage(parser, "usage: test [-h] [-b BETA] alpha") + + @pytest.mark.parametrize("name_mapping_policy", POLICIES) def test_kwargs(name_mapping_policy) -> None: def func(**kwargs) -> str: return f"{kwargs}" parser = _make_parser_for_function(func, name_mapping_policy=name_mapping_policy) - assert_usage(parser, "usage: test [-h]\n") + assert_usage(parser, "usage: test [-h]") @pytest.mark.parametrize( @@ -150,11 +161,11 @@ def func(**kwargs) -> str: [ ( NameMappingPolicy.BY_NAME_IF_HAS_DEFAULT, - "usage: test [-h] [-b BETA] [-d DELTA] alpha gamma\n", + "usage: test [-h] [-b BETA] [-d DELTA] alpha gamma", ), ( NameMappingPolicy.BY_NAME_IF_KWONLY, - "usage: test [-h] -g GAMMA [-d DELTA] alpha [beta]\n", + "usage: test [-h] -g GAMMA [-d DELTA] alpha [beta]", ), ], ) @@ -183,6 +194,8 @@ def _make_parser_for_function( def assert_usage(parser: ArgumentParser, expected_usage: str) -> None: + if not expected_usage.endswith("\n"): + expected_usage += "\n" assert expected_usage == parser.format_usage()