Skip to content

Commit

Permalink
refactor: cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
neithere committed Oct 12, 2023
1 parent f993ca8 commit 69de684
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 116 deletions.
59 changes: 0 additions & 59 deletions src/argh/assembling.py
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,6 @@ def _merge_inferred_and_declared_args(

# arguments inferred from function signature
for parser_add_argument_spec in inferred_args:
# dest = _get_parser_param_kwargs(parser, argspec)["dest"]
specs_by_func_arg_name[
parser_add_argument_spec.func_arg_name
] = parser_add_argument_spec
Expand Down Expand Up @@ -346,64 +345,6 @@ def _merge_inferred_and_declared_args(
return list(specs_by_func_arg_name.values())


def _get_dest(parser: ArgumentParser, argspec: Dict[str, Any]) -> str:
"""
Given a dict representing the keywords to `parser.add_argument()`, extract
and normalise the option name::
>>> _get_dest(parser, {"option_strings": ("-f", "--foo-bar", "--whatever")})
"foo_bar"
"""
kwargs = _get_parser_param_kwargs(parser, argspec)
return kwargs["dest"]


def _get_parser_param_kwargs(
parser: ArgumentParser, argspec: Dict[str, Any]
) -> Dict[str, Any]:
"TODO: explain"
argspec = argspec.copy() # parser methods modify source data
args = argspec.pop("option_strings")

# These two protected methods return something that is, frankly, not very useful:
#
# >>> p._get_positional_kwargs('foo-bar')
# {'required': True, 'dest': 'foo-bar', 'option_strings': []}
#
# >>> p._get_optional_kwargs('foo-bar')
# Traceback (most recent call last):
# File "<stdin>", line 1, in <module>
# File "/usr/lib64/python3.11/argparse.py", line 1571, in _get_optional_kwargs
# raise ValueError(msg % args)
# ValueError: invalid option string 'foo-bar': must start with a character '-'
#
# >>> p._get_positional_kwargs('--foo-bar')
# {'required': True, 'dest': '--foo-bar', 'option_strings': []}
#
# >>> p._get_optional_kwargs('--foo-bar')
# {'dest': 'foo_bar', 'option_strings': ['--foo-bar']}
#
# >>> p._get_optional_kwargs('-a', '-b', '--c', '--d')
# {'dest': 'c', 'option_strings': ['-a', '-b', '-c', '-d']}
#
# Get rid of this. Use function signature as the source of truth.
# (**kwargs allows arbitrary args too.)
# If all are options (`-*`), pick the first elem with `--` (if any) or with `-`,
# strip the prefix and normalise `-` to `_`.
# That's all this thing does,
if _is_positional(args, prefix_chars=parser.prefix_chars):
get_kwargs = parser._get_positional_kwargs
else:
get_kwargs = parser._get_optional_kwargs

kwargs = get_kwargs(*args, **argspec)

kwargs["dest"] = kwargs["dest"].replace("-", "_")

return kwargs


def _is_positional(args: List[str], prefix_chars: str = "-") -> bool:
if not args or not args[0]:
raise ValueError("Expected at least one argument")
Expand Down
51 changes: 3 additions & 48 deletions src/argh/decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
~~~~~~~~~~~~~~~~~~
"""
import warnings
from typing import Callable, List, Optional, Tuple
from typing import Callable, List, Optional

from argh.constants import (
ATTR_ALIASES,
Expand All @@ -23,6 +23,7 @@
ATTR_WRAPPED_EXCEPTIONS_PROCESSOR,
)
from argh.dto import ParserAddArgumentSpec
from argh.utils import naive_guess_func_arg_name, CliArgToFuncArgGuessingError

__all__ = ["aliases", "named", "arg", "wrap_errors", "expects_obj"]

Expand Down Expand Up @@ -136,7 +137,7 @@ def wrapper(func: Callable) -> Callable:
if not args:
raise CliArgToFuncArgGuessingError("at least one CLI arg must be defined")

func_arg_name = _naive_guess_func_arg_name(args)
func_arg_name = naive_guess_func_arg_name(args)
completer = kwargs.pop("completer", None)
spec = ParserAddArgumentSpec.make_from_kwargs(
func_arg_name=func_arg_name,
Expand Down Expand Up @@ -237,49 +238,3 @@ def foo(bar, quux=123):
)
setattr(func, ATTR_EXPECTS_NAMESPACE_OBJECT, True)
return func


def _naive_guess_func_arg_name(option_strings: Tuple[str, ...]) -> str:
def _opt_to_func_arg_name(opt: str) -> str:
return opt.strip("-").replace("-", "_")

if len(option_strings) == 1:
# the only CLI arg name; adapt and use
return _opt_to_func_arg_name(option_strings[0])

are_args_positional = [not arg.startswith("-") for arg in option_strings]

if any(are_args_positional) and not all(are_args_positional):
raise MixedPositionalAndOptionalArgsError

if all(are_args_positional):
raise TooManyPositionalArgumentNames

for option_string in option_strings:
if not option_string.startswith("-"):
# not prefixed; use as is
return option_strings[0]

if option_string.startswith("--"):
# prefixed long; adapt and use
return _opt_to_func_arg_name(option_string[2:])

raise CliArgToFuncArgGuessingError(
f"Unable to convert opt strings {option_strings} to func arg name"
)


class ArghError(Exception):
...


class CliArgToFuncArgGuessingError(ArghError):
...


class TooManyPositionalArgumentNames(CliArgToFuncArgGuessingError):
...


class MixedPositionalAndOptionalArgsError(CliArgToFuncArgGuessingError):
...
48 changes: 47 additions & 1 deletion src/argh/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
import argparse
import inspect
import re
from typing import Callable
from typing import Callable, Tuple


def get_subparsers(
Expand Down Expand Up @@ -79,3 +79,49 @@ def unindent(text: str) -> str:

class SubparsersNotDefinedError(Exception):
...


def naive_guess_func_arg_name(option_strings: Tuple[str, ...]) -> str:
def _opt_to_func_arg_name(opt: str) -> str:
return opt.strip("-").replace("-", "_")

if len(option_strings) == 1:
# the only CLI arg name; adapt and use
return _opt_to_func_arg_name(option_strings[0])

are_args_positional = [not arg.startswith("-") for arg in option_strings]

if any(are_args_positional) and not all(are_args_positional):
raise MixedPositionalAndOptionalArgsError

if all(are_args_positional):
raise TooManyPositionalArgumentNames

for option_string in option_strings:
if not option_string.startswith("-"):
# not prefixed; use as is
return option_strings[0]

if option_string.startswith("--"):
# prefixed long; adapt and use
return _opt_to_func_arg_name(option_string[2:])

raise CliArgToFuncArgGuessingError(
f"Unable to convert opt strings {option_strings} to func arg name"
)


class ArghError(Exception):
...


class CliArgToFuncArgGuessingError(ArghError):
...


class TooManyPositionalArgumentNames(CliArgToFuncArgGuessingError):
...


class MixedPositionalAndOptionalArgsError(CliArgToFuncArgGuessingError):
...
16 changes: 8 additions & 8 deletions tests/test_decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@
import pytest

import argh
from argh.decorators import (
from argh.dto import ParserAddArgumentSpec
from argh.utils import (
CliArgToFuncArgGuessingError,
MixedPositionalAndOptionalArgsError,
TooManyPositionalArgumentNames,
_naive_guess_func_arg_name,
naive_guess_func_arg_name,
)
from argh.dto import ParserAddArgumentSpec


def test_aliases():
Expand Down Expand Up @@ -90,17 +90,17 @@ def test_naive_guess_func_arg_name() -> None:
argh.arg()(lambda foo: foo)

# positional
assert _naive_guess_func_arg_name(("foo",)) == "foo"
assert naive_guess_func_arg_name(("foo",)) == "foo"

# positional — more than one (error)
with pytest.raises(TooManyPositionalArgumentNames):
argh.arg("foo", "bar")(lambda foo: foo)

# option
assert _naive_guess_func_arg_name(("--foo",)) == "foo"
assert _naive_guess_func_arg_name(("--foo", "-f")) == "foo"
assert _naive_guess_func_arg_name(("-f", "--foo")) == "foo"
assert _naive_guess_func_arg_name(("-x", "--foo", "--bar")) == "foo"
assert naive_guess_func_arg_name(("--foo",)) == "foo"
assert naive_guess_func_arg_name(("--foo", "-f")) == "foo"
assert naive_guess_func_arg_name(("-f", "--foo")) == "foo"
assert naive_guess_func_arg_name(("-x", "--foo", "--bar")) == "foo"

# mixed (errors)
with pytest.raises(MixedPositionalAndOptionalArgsError):
Expand Down

0 comments on commit 69de684

Please sign in to comment.