Create '--non-interactive' option for setup_openfold#264
Conversation
| monkeypatch.delenv("OPENFOLD_CACHE", raising=False) | ||
| monkeypatch.setenv("HOME", str(tmp_path)) |
There was a problem hiding this comment.
nit: I generally don't try to mix different testing apis/styles, so either monkey patch or patch but not both
| """Main execution.""" | ||
| @click.command() | ||
| @click.option( | ||
| "--run-with-defaults", |
There was a problem hiding this comment.
a more typical flag would be '--quiet' or '--non-interactive'
There was a problem hiding this comment.
replaced with --non-interactive
| openfold_cache, ckpt_root_file = setup_openfold_cache() | ||
| openfold_cache, ckpt_root_file = setup_openfold_cache( | ||
| run_with_defaults=run_with_defaults | ||
| ) | ||
|
|
||
| # Step 2: Set up checkpoint directory | ||
| param_dir, should_download = setup_param_directory(openfold_cache, ckpt_root_file) | ||
| param_dir, should_download = setup_param_directory( | ||
| openfold_cache, ckpt_root_file, run_with_defaults=run_with_defaults | ||
| ) | ||
|
|
||
| # Step 3: Perform download if needed | ||
| if should_download: | ||
| download_parameters(param_dir) | ||
| download_parameters(param_dir, run_with_defaults=run_with_defaults) | ||
|
|
||
| # Step 4: Setup CCD with biotite | ||
| setup_biotite_ccd(ccd_path=biotite.setup_ccd.OUTPUT_CCD, force_download=False) | ||
|
|
||
| # Step 5: Run tests (always run regardless of download status) | ||
| run_integration_tests() | ||
| run_integration_tests(run_with_defaults=run_with_defaults) |
There was a problem hiding this comment.
This is perhaps the most structural comment (and serious) rather than replicating the boolean flag (these tend to multiply) we pull this out and create a config object, if it's interactive, it asks user how to configure, otherwise it constructs a default one, and we just pass the config object down?
Will break if we need to do a lot of checking if things exist/don't exist when constructing the config
There was a problem hiding this comment.
PTAL, based on this comment, I made a heavier refactor of setup_openfold. I think it will be worth it / make the options more clear.
| ) | ||
| with env_patch, s3_patch: | ||
| os.environ.pop("OPENFOLD_CACHE", None) | ||
| result = CliRunner().invoke(setup_openfold.main, ["--non-interactive"]) |
| """Set up OpenFold3 model parameters.""" | ||
| if config_path is not None: | ||
| config = OpenFoldSetupConfig.model_validate_json(config_path.read_text()) | ||
| elif non_interactive: | ||
| config = OpenFoldSetupConfig() | ||
| else: | ||
| config = _prompt_for_config() |
Summary
setup_openfoldis currently an interactive script that asks 4 questions regarding where to download the model parameters, which model parameter(s) to download, and whether to run integration tests.For automated workflows, it is convenient to provide a "--non-interactive" option for the script such that all default options are automatically selected. One immediate use case is in TestSetupOpenfold (in test_entry_points.py), where we need to provide fake values for setup_openfold.main(). Another use case is in example colabfold notebooks, such as #255
Changes
OpenFoldSetupConfigpydantic model to record options, and store default settings. Interactive mode now populates OpenFoldSetupConfig with user input through the same set of queries provided previously.--non-interactiveoption, which will use the default settings ofOpenFoldSetupConfigtest_entry_points.TestSetupOpenFoldwith additional test to test default settings and incorporate cli_runner.Related Issues
#255
Testing
test_entry_points.pytests passtest_inference_full.pypasses