Unify defaults handling for click cli

[wresch]

I noticed that the defaults stated in the CLI help strings did not match up with actual defaults for

• --max_parallel_samples None vs 5
• --subsample_msa True vs False
• --write_full_pae True vs False
• --preprocessing-threads 1 vs multiprocessing.cpu_count()

and --method did not have a list of possible options.

The defaults are currently defined in three separate places (a manual help string, defaults for the click options, and defaults for the function's arguments) which makes it hard to reason about and maintain. This PR proposes to unify defaults handling by making sure all options have a default, there are on/of flags for bool options, and click's show_default is used to automatically show defaults instead of using the manually added default in the help string.

Note that the PR change the actual default of subsample_msa to True as implied in the help string. The other options stay as they were actually implemented. I'm not sure that was the right approach.

[jwohlwend]

Thank you, I agree with the general objective of the PR! Couple of follow-up questions:

• subsample_msa should be False by default
• Another question I have is whether this affect calling the predict function in library mode? I believe it does

[wresch]

I think calling a click-wrapped function from code is kind of awkward anyway since it's now a wrapped click command. How about something like this:

import click
from dataclasses import dataclass

@dataclass
class PredictConfig:
    subsample_msa: bool = False

@click.group()
def cli() -> None:
    """Boltz."""
    return

@cli.command("predict", context_settings={'show_default': True})
@click.option(
    "--subsample_msa/--no_subsample_msa",
    is_flag=True,
    help="Whether to subsample the MSA.",
    default=False,
)
def _predict(subsample_msa: bool):
    config = PredictConfig(**locals())
    predict(config)


def predict(config: PredictConfig) -> None:
    """Predict function."""
    print(f"Subsample MSA: {config.subsample_msa}")


if __name__ == "__main__":
    cli()

It would (1) separate the cli from the backend logic and (2) make it easy/idiomatic to call the predict function. I threw in a config object because it looks like you use that in other places but that would also require changes to the predict function to extract the arguments from the object. But it would make the interface to predict simpler. And you could or could not include defaults in the config object. Since the click arguments now define defaults for all values it wouldn't make a difference.

Let me know what you would prefer

[wresch]

the config object was just a thought. If you wanted to make no changes to the underlying predict function we could use

import click

@click.group()
def cli() -> None:
    """Boltz."""
    return

@cli.command("predict", context_settings={'show_default': True})
@click.option(
    "--subsample_msa/--no_subsample_msa",
    is_flag=True,
    help="Whether to subsample the MSA.",
    default=False,
)
def _predict(subsample_msa: bool):
    predict(**locals())


def predict(subsample_msa: bool) -> None:
    """Predict function."""
    print(f"Subsample MSA: {config.subsample_msa}")


if __name__ == "__main__":
    cli()```

and again - with or without providing defaults for the arguments to `predict`

[wresch]

I don't have any experience with it but there is also https://github.com/couling/dataclass-click if you wanted have a single dataclass define your CLI and your api function.

[wresch]

Let me know what you think works best for your project and I'll update the PR