Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Request: Support Actions #71

Open
North101 opened this issue Feb 7, 2024 · 3 comments · May be fixed by #72
Open

Request: Support Actions #71

North101 opened this issue Feb 7, 2024 · 3 comments · May be fixed by #72

Comments

@North101
Copy link

North101 commented Feb 7, 2024

I want to have an argument that takes multiple values but of different types. This seems only possible with the action argument which isn't supported by typed-argparse.

In my fork I've added the action argument and everything works as is, though I think it might require a little more thought than just that. (e.g. which args should allow action)

@bluenote10
Copy link
Collaborator

I want to have an argument that takes multiple values but of different types. This seems only possible with the action argument which isn't supported by typed-argparse.

Could you elaborate a little bit on the use case, i.e., give a small example of what the feature would look like?

If you're referring to the store_const action: Why not simply define a derived @property on the arguments class that converts the underlying boolean argument into a T | None (i.e., the property getter either returns T if the argument --foo was specified, and None if it wasn't).

Since it is typically possible to solve such use cases in a type-safe fashion on top of the raw arguments, I haven't really encountered a use case where I truly needed to expose the action API. Exposing it could indeed get very subtle when trying to cover all edge cases.

@North101
Copy link
Author

North101 commented Feb 12, 2024

I have an argument --image which is optional and has nargs=4. The 4 args and their types are:

  • path: pathlib.Path
  • width: float
  • height: float
  • scale: float

This isn't possible as nargs expects a list all of the same type. So i've used action= to replace the parsing logic to allow returning a NamedTuple and using the types of that NamedTuple to parse the types.

class Image(NamedTuple):
  path: pathlib.Path
  width: float
  height: float
  scale: float


def is_NamedTuple(obj) -> bool:
  return (
      tuple in obj.__bases__ and
      hasattr(obj, '_fields') and
      hasattr(obj, '__annotations__')
  )


class NamedTupleAction(argparse.Action):
  def __init__(self, option_strings: list[str], dest: str, type: Any, nargs=None, metavar=None, **kwargs):
    assert is_NamedTuple(type), f'{type} is not a NamedTuple'
    assert nargs is None, 'nargs is not None'
    assert metavar is None, 'metavar is not None'

    metavar = tuple((
        field.upper()
        for field in type._fields
    ))
    super().__init__(option_strings, dest, type=None, nargs=len(metavar), metavar=metavar, **kwargs)

    self._type = type

  def field_type_at_index(self, index: int):
    field = self._type._fields[index]
    return self._type.__annotations__[field]

  def parse_value(self, index: int, value: str):
    subtype = self.field_type_at_index(index)
    try:
      return subtype(value)
    except (TypeError, ValueError):
      name = getattr(subtype, '__name__', repr(subtype))
      args = {'type': name, 'value': value}
      msg = gettext('invalid %(type)s value: %(value)r')
      raise argparse.ArgumentError(self, msg % args)

  def parse_values(self, values: Sequence[str]):
    return self._type(*(
        self.parse_value(i, value)
        for i, value in enumerate(values)
    ))

  def __call__(self, parser, namespace, values, option_string=None):
    if not isinstance(values, list):
      raise ValueError(values)

    setattr(namespace, self.dest, self.parse_values(values))


class CustomArgs(tap.TypedArgs):
  image: Image | None = tap.arg(
      default=None,
      action=types.NamedTupleAction,
  )

@bluenote10
Copy link
Collaborator

That's a pretty interesting use case indeed!

I'm not having a lot of time right now to look into it in detail and will be offline for a few weeks, so just a few thoughts for now.

On first glance it could make sense to handle the case where T is a tuple implicitly as a fixed nargs corresponding to the size of the tuple. All the low-level implementation details how this is forward to argparse (using actions) could then be hidden from the user if it "just works". One has to be a little bit careful though to avoid resulting in an ambiguous parser. In particular if the tuple would involve options like (int, str, float | None) the nargs would be 2 or 3. I'm not sure if something like that would make sense (and if argparse would even allow that) because the variable size would be ambiguous with regular position arguments. For instance, in --my-fancy-tuple <arg_a> <arg_b> <arg_c> the <arg_c> could either be parse as the third argument of --my-fancy-tuple or as position argument to the main parser.

@North101 North101 linked a pull request Feb 14, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants