lebrice / SimpleParsing

Simple, Elegant, Typed Argument Parsing with argparse
MIT License
399 stars 49 forks source link

force_full_path option to use always full paths to fields as option string #117

Closed ivanprado closed 2 years ago

ivanprado commented 2 years ago

This PR is an attempt to provide the functionality at https://github.com/lebrice/SimpleParsing/issues/75

I think this functionality is really useful. Indeed, in my particular use case, it is crucial.

Probably I'm missing something in the code. So I'm very happy to receive feedback about it :-)

ivanprado commented 2 years ago

Thank you @lebrice . I'm going to work on your feedback. Meanwhile, here I have the answer to your question:

Can you give me an example of what kind of use-case there would be for FULL_WITHOUT_ROOT

Imagine you want to control all the argument parsing using dataclasses for a server that is serving a machine learning model. The following could be the options dataclasses:

@dataclass
class ModelOptions:
  path: Path
  device: str

@dataclass
class ServerOptions:
  host: str
  port: int
  model: ModelOptions

# There is a function that receives the ServerOptions and starts a server
def start_server(options: ServerOptions):
  ...

parser = ArgumentParser()
parser.add_arguments(ServerOptions, dest='options')

The default simple-parsing generated arguments would then be --host str --port int --path Path --device str. Note how path is ambiguous here. Path to what? We could use the new option to create nested args instead using the option `FullPathMode.FULL. The arguments would then be:

--options.host str --options.port int  --options.model.path Path --options.model.device str

Now the path is not ambiguous now. It is clearly the path to the model. But all arguments contains now the redundant word options. This is where the option FullPathMode.FULL_WITHOUT_ROOT comes to the rescue. It generates the following arguments:

--host str --port int --model.path Path --model.device

The redundant options. prefix has been removed. The first level is plain, and the rest of the levels contains the path of the argument. In summary:

ivanprado commented 2 years ago

I think the add_dest_to_option_strings argument is overlapping with this new option, and we should probably fuse them both into a new option instead.

@lebrice I've fused add_dest_to_option_strings with this new feature. So now this is control with to arguments:

Could you have a look? Thanks you :-)

ivanprado commented 2 years ago

@lebrice I've addressed your comments. Ready for a re-review.

ivanprado commented 2 years ago

@lebrice thank you to you for accepting my contribution :-)