From 9463459be1f92473ce874df1cf0c4a327cc2e514 Mon Sep 17 00:00:00 2001 From: Andy Mikhaylenko Date: Mon, 23 Oct 2023 16:04:58 +0200 Subject: [PATCH 1/6] fix: regression "TypeError: cannot pickle..." (fixes #204) --- CHANGES.rst | 8 ++++++++ pyproject.toml | 2 +- src/argh/assembling.py | 18 ++++++------------ tests/test_regressions.py | 19 +++++++++++++++++++ 4 files changed, 34 insertions(+), 13 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 654cc5e..1e509cd 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -2,6 +2,14 @@ Changelog ~~~~~~~~~ +Version 0.30.1 +-------------- + +Bugs fixed: + +- regression: certain special values in argument default value would cause an + exception (#204) + Version 0.30.0 -------------- diff --git a/pyproject.toml b/pyproject.toml index b55c8a7..04f6c2f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "flit_core.buildapi" [project] name = "argh" -version = "0.30.0" +version = "0.30.1" 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 d57d48e..db17a6a 100644 --- a/src/argh/assembling.py +++ b/src/argh/assembling.py @@ -16,7 +16,6 @@ import inspect from argparse import OPTIONAL, ZERO_OR_MORE, ArgumentParser from collections import OrderedDict -from dataclasses import asdict from enum import Enum from typing import Any, Callable, Dict, Iterator, List, Optional, Tuple @@ -315,9 +314,9 @@ def set_default_command( raise AssemblingError(f"{function.__name__}: {exc}") from exc # add the fully formed argument specs to the parser - for orig_spec in parser_add_argument_specs: - spec = _prepare_parser_add_argument_spec( - parser_add_argument_spec=orig_spec, parser_adds_help_arg=parser.add_help + for spec in parser_add_argument_specs: + _extend_parser_add_argument_spec( + spec=spec, parser_adds_help_arg=parser.add_help ) try: @@ -347,12 +346,9 @@ def set_default_command( ) -def _prepare_parser_add_argument_spec( - parser_add_argument_spec: ParserAddArgumentSpec, parser_adds_help_arg: bool -) -> ParserAddArgumentSpec: - # deep copy - spec = ParserAddArgumentSpec(**asdict(parser_add_argument_spec)) - +def _extend_parser_add_argument_spec( + spec: ParserAddArgumentSpec, parser_adds_help_arg: bool +) -> None: # add types, actions, etc. (e.g. default=3 implies type=int) spec.other_add_parser_kwargs.update( guess_extra_parser_add_argument_spec_kwargs(spec) @@ -369,8 +365,6 @@ def _prepare_parser_add_argument_spec( if parser_adds_help_arg and "-h" in spec.cli_arg_names: spec.cli_arg_names = [name for name in spec.cli_arg_names if name != "-h"] - return spec - def _merge_inferred_and_declared_args( inferred_args: List[ParserAddArgumentSpec], diff --git a/tests/test_regressions.py b/tests/test_regressions.py index 5d10cdf..eaa3a6d 100644 --- a/tests/test_regressions.py +++ b/tests/test_regressions.py @@ -2,6 +2,9 @@ Regression tests ~~~~~~~~~~~~~~~~ """ +import sys +from typing import TextIO + import pytest import argh @@ -150,3 +153,19 @@ def cmd(foo_foo, bar_bar, *, baz_baz=5, bip_bip=9, **kwargs): parser.set_default_command(cmd) expected = "abc\ndef\n8\n9\n{}\n" assert run(parser, "abc def --baz-baz 8").out == expected + + +def test_regression_issue204(): + """ + Issue #204: `asdict(ParserAddArgumentSpec)` used `deepcopy` which would + lead to "TypeError: cannot pickle..." if e.g. a default value contained an + un-pickle-able object. + + We should avoid `deepcopy()` in standard operations. + """ + + def func(x: TextIO = sys.stdout) -> None: + ... + + parser = DebugArghParser() + parser.set_default_command(func) From b3cb805ea0545eba0b1602e0e329d5bbcf051de6 Mon Sep 17 00:00:00 2001 From: Andy Mikhaylenko Date: Mon, 23 Oct 2023 16:12:56 +0200 Subject: [PATCH 2/6] chore: add marker file for PEP-561 (typing) --- src/argh/py.typed | 1 + 1 file changed, 1 insertion(+) create mode 100644 src/argh/py.typed diff --git a/src/argh/py.typed b/src/argh/py.typed new file mode 100644 index 0000000..cf15595 --- /dev/null +++ b/src/argh/py.typed @@ -0,0 +1 @@ +# Marker file for PEP-561 From 778f3e875f9dffffb3ed2f564645af1111b3ec62 Mon Sep 17 00:00:00 2001 From: Andy Mikhaylenko Date: Sun, 22 Oct 2023 02:38:25 +0200 Subject: [PATCH 3/6] docs: improve the tutorial --- docs/source/tutorial.rst | 139 ++++++++++++++++++++++++++------------- 1 file changed, 94 insertions(+), 45 deletions(-) diff --git a/docs/source/tutorial.rst b/docs/source/tutorial.rst index 03495cd..aa6ac12 100644 --- a/docs/source/tutorial.rst +++ b/docs/source/tutorial.rst @@ -17,7 +17,7 @@ Assume we need a CLI application which output is modulated by arguments: $ ./greet.py Hello unknown user! - $ ./greet.py --name John + $ ./greet.py John Hello John! This is our business logic: @@ -34,6 +34,19 @@ Let's convert the function into a complete CLI application:: Done. Dead simple. +You may want to make the name an "option" AKA named CLI argument, like this:: + + $ ./greet.py --name John + +In that case it's enough to make the function argument `name` "keyword-only" +(see :pep:`3102` for explanation):: + + def main(*, name: str = "unknown user") -> str: + ... + +Everything to the left of ``*`` becomes a positional CLI argument. Everything +to the right of ``*`` becomes a named one. + What about multiple commands? Easy:: argh.dispatch_commands([load, dump]) @@ -75,71 +88,102 @@ Declaring Commands The Natural Way ............... -You've already learned the natural way of declaring commands before even -knowing about `argh`:: +If you are comfortable with the basics of Python, you already knew the natural +way of declaring CLI commands with `Argh` before even learning about the +existence of `Argh`. + +Please read the following snippet carefully. Is there any `Argh`-specific API? + +:: + + def my_command( + alpha: str, beta: int = 1, *args, gamma: int, delta: bool = False + ) -> list[str]: + return [alpha, beta, args, gamma, delta] - def my_command(alpha, beta=1, gamma=False, *delta): - return +The answer is: no. This is a completely generic Python function. -When executed as ``app.py my-command --help``, such application prints:: +Let's make this function available as a CLI command:: - usage: app.py my-command [-h] [-b BETA] [-g] alpha [delta ...] + import argh + + + def my_command( + alpha: str, beta: int = 1, *args, gamma: int, delta: bool = False + ) -> list[str]: + return [alpha, beta, args, gamma, delta] + + + if __name__ == "__main__": + argh.dispatch_commands([my_command]) + +That's all. You don't need to do anything else. + +When executed as ``./app.py my-command --help``, such application prints:: + + usage: app.py my-command [-h] -g GAMMA [-d] alpha [beta] [args ...] positional arguments: - alpha - delta + alpha - + beta 1 + args - - optional arguments: + options: -h, --help show this help message and exit - -b BETA, --beta BETA - -g, --gamma + -g GAMMA, --gamma GAMMA + - + -d, --delta False -The same result can be achieved with this chunk of `argparse` code (with the -exception that in `argh` you don't immediately modify a parser but rather -declare what's to be added to it later):: +Now let's take a look at how we would do it without `Argh`:: - parser.add_argument("alpha") - parser.add_argument("-b", "--beta", default=1, type=int) - parser.add_argument("-g", "--gamma", default=False, action="store_true") - parser.add_argument("delta", nargs="*") + import argparse -Verbose, hardly readable, requires learning another API. -`Argh` allows for more expressive and pythonic code because: + def my_command( + alpha: str, beta: int = 1, *args, gamma: int, delta: bool = False + ) -> list[str]: + return [alpha, beta, args, gamma, delta] -* everything is inferred from the function signature; -* arguments without default values are interpreted as required positional - arguments; -* arguments with default values are interpreted as options; - * options with a `bool` as default value are considered flags and their - presence triggers the action `store_true` (or `store_false`); - * values of options that don't trigger actions are coerced to the same type - as the default value; + if __name__ == "__main__": + parser = argparse.ArgumentParser() -* the ``*args`` entry (function's positional arguments) is interpreted as - a single argument with 0..n values. + subparser = parser.add_subparsers().add_parser("my-command") -Hey, that's a lot for such a simple case! But then, that's why the API feels -natural: `argh` does a lot of work for you. + subparser.add_argument("alpha") + subparser.add_argument("beta", default=1, nargs="?", type=int) + subparser.add_argument("args", nargs="*") + subparser.add_argument("-g", "--gamma") + subparser.add_argument("-d", "--delta", default=False, action="store_true") -.. note:: + ns = parser.parse_args() - The pattern described above is the "by name if has default" mapping policy. - It used to be *the* policy but starting with Argh v.0.30 there's a better - one, "by name if kwonly". Although the older one will remain the default - policy for a while, it may be eventually dropped in favour of the new one. + lines = my_command(ns.alpha, ns.beta, *ns.args, gamma=ns.gamma, delta=ns.delta) - Please check `~argh.assembling.NameMappingPolicy` for details. + for line in lines: + print(line) - Usage example:: +Verbose, hardly readable, requires learning the API. With `Argh` it's just a +single line in addition to your function. - def my_command(alpha, beta=1, *, gamma, delta=False, **kwargs): - ... +`Argh` allows for more expressive and pythonic code because: + +* everything is inferred from the function signature; +* regular function arguments are represented as positional CLI arguments; +* varargs (``*args``) are represented as a "zero or more" positional CLI argument; +* kwonly (keyword-only arguments, see :pep:`3102`) are represented as named CLI + arguments; + + * keyword-only arguments with a `bool` default value are considered flags + (AKA toggles) and their presence triggers the action `store_true` (or + `store_false`). - argh.dispatch_command( - my_command, name_mapping_policy=NameMappingPolicy.BY_NAME_IF_KWONLY - ) +* you can ``print()`` but you don't have to — the return value will be printed + for you; it can even be an iterable (feel free to ``yield`` too), then each + element will be printed on its own line. + +Hey, that's a lot for such a simple case! But then, that's why the API feels +natural: `argh` does a lot of work for you. Well, there's nothing more elegant than a simple function. But simplicity comes at a cost in terms of flexibility. Fortunately, `argh` doesn't stay in @@ -158,6 +202,11 @@ Extended argument declaration can be helpful in that case. Extended Argument Declaration ............................. +.. note:: + + This section will be out of date soon. Typing hints will be used for all + the cases described here including argument help. + When function signature isn't enough to fine-tune the argument declarations, the :class:`~argh.decorators.arg` decorator comes in handy:: From b65e65c385bdb37565f399adfd5f2d17834ecdbe Mon Sep 17 00:00:00 2001 From: Andy Mikhaylenko Date: Mon, 23 Oct 2023 16:16:41 +0200 Subject: [PATCH 4/6] docs: update changelog --- CHANGES.rst | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index 1e509cd..d22497c 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -7,9 +7,17 @@ Version 0.30.1 Bugs fixed: -- regression: certain special values in argument default value would cause an +- Regression: certain special values in argument default value would cause an exception (#204) +Enhancements: + +- Improved the tutorial. + +Other changes: + +- Added `py.typed` marker file for :pep:`561`. + Version 0.30.0 -------------- From 928aa1e77fc1c70b7e2237e59f188585a6e93de9 Mon Sep 17 00:00:00 2001 From: Andy Mikhaylenko Date: Mon, 23 Oct 2023 16:29:11 +0200 Subject: [PATCH 5/6] feat: more informative error message --- src/argh/assembling.py | 5 ++++- tests/test_regressions.py | 4 +++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/argh/assembling.py b/src/argh/assembling.py index db17a6a..93c7f19 100644 --- a/src/argh/assembling.py +++ b/src/argh/assembling.py @@ -415,7 +415,10 @@ def _merge_inferred_and_declared_args( kind_declared = kinds[decl_positional] raise AssemblingError( f'argument "{func_arg_name}" declared as {kind_inferred} ' - f"(in function signature) and {kind_declared} (via decorator)" + f"(in function signature) and {kind_declared} (via decorator). " + "If you've just migrated from Argh v.0.29, please check " + "the new default NameMappingPolicy. Perhaps you need " + "to replace `func(x=1)` with `func(*, x=1)`?" ) # merge explicit argument declaration into the inferred one diff --git a/tests/test_regressions.py b/tests/test_regressions.py index eaa3a6d..6e4314b 100644 --- a/tests/test_regressions.py +++ b/tests/test_regressions.py @@ -116,7 +116,9 @@ def func(foo_bar): parser.set_default_command(func) msg = ( 'func: argument "foo_bar" declared as positional (in function ' - "signature) and optional (via decorator)" + "signature) and optional (via decorator). If you've just migrated " + "from Argh v.0.29, please check the new default NameMappingPolicy. " + "Perhaps you need to replace `func(x=1)` with `func(*, x=1)`?" ) assert excinfo.exconly().endswith(msg) From ee0bc82c9402aa12d4bcf5fe336bb21dc21af8a6 Mon Sep 17 00:00:00 2001 From: Andy Mikhaylenko Date: Mon, 23 Oct 2023 16:45:26 +0200 Subject: [PATCH 6/6] docs: update changelog --- CHANGES.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES.rst b/CHANGES.rst index d22497c..a6aa698 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -13,6 +13,9 @@ Bugs fixed: Enhancements: - Improved the tutorial. +- Added a more informative error message when the reason is likely to be + related to the migration from Argh v0.29 to a version with a new argument + name mapping policy. Other changes: