czbiohub-sf / shrimPy

shrimPy: Smart High-throughput Robust Imaging & Measurement in Python
BSD 3-Clause "New" or "Revised" License
8 stars 1 forks source link

Should the output and config CLI calls have a default option? #92

Closed edyoshikun closed 1 year ago

edyoshikun commented 1 year ago

This is not a top priority at the moment, but it would be nice if we can have a default string/path that can be set for the config and the output path filepath. Would this be something that is worth implementing at some point? I do find it that most of the times the registration.yml or the deskew.yml end up being named the same thing. Perhaps this is something that can get implemented when we slowly migrate things to pathlib.Path. @talonchandler @ieivanov

For example:

def output_filepath(default=None) -> Callable:
    def decorator(f: Callable) -> Callable:
        option_kwargs = {
            "param_decls": ["--output-filepath", "-o"],
            "required": True,
            "type": click.Path(exists=False, file_okay=True, dir_okay=False),
            "help": "Path to output file",
        }

        if default is not None:
            option_kwargs["default"] = default

        return click.option(**option_kwargs)(f)

    return decorator

@output_filepath(default='./deskew.yml')
@click.command()
def my_command(output_filepath):
    click.echo(f"Output filepath: {output_filepath}")
ieivanov commented 1 year ago

I personally don't think it's a good idea - I think you should always have to make a conscious decision about how you're reconstructing the data and where you're saving it. Autocomplete on your terminal could get around some of the headache of having to type it out every time if you're using consistent naming.

talonchandler commented 1 year ago

I'll vote this @ieivanov on this one. Autocomplete helps, and I expect this problem to disappear as our pipeline reaches stability and we start running complete analyses with few-line scripts instead of semi-manually iterating CLI calls step by step.

edyoshikun commented 1 year ago

Sounds good, you guys convinced me and we have everyone's consensus. Closing this issue as autocomplete will certainly solve the issue. Thank you both for your input.