lebrice / SimpleParsing

Simple, Elegant, Typed Argument Parsing with argparse
MIT License
427 stars 52 forks source link

Support for Positional Arguments #89

Closed haydenflinner closed 3 years ago

haydenflinner commented 3 years ago

Is your feature request related to a problem? Please describe. It would be nice to be able to have positional arguments, especially for cmds that only take one argument. For example, given the following cmd, it would be annoying to always have to instead type: cat myfile | sed 's/abc/123/g' cat --file myfile | sed --pattern 's/abc/123/g'.

Describe the solution you'd like I think I just need a way to tell 'options_strings' not to add the prefix dashes. If the option name is specified to argparse without dashes, it is positional by default.

lebrice commented 3 years ago

Interesting! You mean within a dataclass, right? (Because otherwise you might as well just add the positional argument the old fashion way with argparse)

haydenflinner commented 3 years ago

Right; the normal add argument interface is quite annoying, and I'd much prefer doing everything through SimpleParsing. So I actually don't even use the normal interface at all; I have a decorator which turns any function into an equivalent one which accepts a dataclass instance instead.

On Thu, Oct 7, 2021, 9:29 PM Fabrice Normandin @.***> wrote:

Interesting! You mean within a dataclass, right? (Because otherwise you might as well just add the positional argument the old fashion way with argparse)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/lebrice/SimpleParsing/issues/89#issuecomment-938288595, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADRIK3CN6N4JLKWS7WZKWRLUFZJPDANCNFSM5FSA4HRA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

lebrice commented 3 years ago

Oh interesting! Would you mind checking if the field function from simple_parsing.helpers accepts a "option_strings" argument? (Can't recall from memory).

If not, then whomever (not sure how to use this word, sounds fancy) stumbles upon this and has a bit of free time: We could add such an argument to field (a bit like the aliases one that already exists) and just use it the same way as aliases in the FieldWrapper, but instead of adding it to the field name to get the option_strings, that argument would overwrite it and be used as self.option_strings

lebrice commented 3 years ago

Also @haydenflinner I'm curious, would you mind sharing an example of the wrapper function you described?

haydenflinner commented 3 years ago

Rudimentary support for positional implemented here.

--

The wrapper function I described is part of my own attempt at this same problem space called simpcli3. When it came time to implement parsing nested dataclasses within Unions I gave up and chose to wrap SimpleParsing instead :smile: . Usage looks like this:

from simpcli3 import CliApp
from typing import List

def myls(paths: List[str], exclude: List[str]=[], mystr: str=None, follow_symlinks: bool=False):
    print(f"Received args: {paths}\n")
    for path in paths:
        print(path)

if __name__ == "__main__":
    CliApp(myls).run()

and I want to be able to call it like this (note the no-prefix on follow_symlinks done automatically, don't have to call flag() explicitly) ls.py --path x --exclude y --no-follow-symlinks --print-format LINE_PER_ENTRY'

The wrapper is as follows:

class CallingInfo:
    def __init__(self, func):
        self._func = func

        self._expand_dataclass_arg = False
        self._dataclass = None

        sig = signature(func)
        if not sig.parameters:
            return
        first_param = next(iter(sig.parameters.values()), None)
        if is_dataclass(first_param.annotation):
            # def myfunc(all_of_my_args: MyArgType):
            self._dataclass = first_param.annotation
            return

        # If we make it down to here, it's epxected that we've wrapped a function like this:
        # def myfunc(arg1, arg2='hi',  arg3=True, arg4=4):
        # Let's try to translate this to a dataclass and re-use our prior code for argparsing dataclasses.
        dataclass = _sig_to_params_dataclass(func.__name__, sig)
        self._expand_dataclass_arg = True
        self._dataclass = dataclass

    def call(self, dataclass_arg):
        if self._expand_dataclass_arg:
            self._func(**dataclasses.asdict(datclass_arg))
        else:
            self._func(dataclass_arg)

def func_to_params_dataclass(func):
    sig = signature(func)
    return _sig_to_params_dataclass(func.__name__, sig)

import dataclasses
def _sig_to_params_dataclass(func_name, sig):
    def _get_default_factory(p):
        # Workaround for what might be Python's most annoying gotcha.
        return lambda: p.default
    dc_params = []
    for param in sig.parameters.values():
        if param.annotation is sig.empty:
            raise TypeError(f"Will not guess parameter types, please annotate type for param {param.name!r}.")
        if param.default is not sig.empty:
            # I don't think there is any downside to always using default_factory rather than trying to use default,
            # and only using default_factory in the cases where dataclasses would throw a ValueError.
            dc_params.append((param.name, param.annotation, dataclasses.field(default_factory=_get_default_factory(param))))
        else:
            dc_params.append((param.name, param.annotation))

    returning = dataclasses.make_dataclass(f'{func_name.capitalize()}Args', dc_params)
    logger.debug(f"Made dataclass: {func_name}: {dc_params}")
    return returning
lebrice commented 3 years ago

Cool! Could you perhaps make a pull request for the positional args feature?

haydenflinner commented 3 years ago

91

lebrice commented 3 years ago

Closing this.