From 915d11022c31ce5e6835aa220965eb6c5698f794 Mon Sep 17 00:00:00 2001 From: Alyssa Travitz <31974495+atravitz@users.noreply.github.com> Date: Tue, 14 Jan 2025 08:50:37 -0800 Subject: [PATCH] Support outdir created at runtime (#1072) * adding test * create directories * fixing a word * update news * update news --- news/986_input_validation.rst | 4 ++-- news/output_file_handling.rst | 23 +++++++++++++++++++++ openfecli/commands/quickrun.py | 16 ++++++++------- openfecli/parameters/output.py | 15 -------------- openfecli/tests/commands/test_quickrun.py | 25 ++++++++++++++++------- 5 files changed, 52 insertions(+), 31 deletions(-) create mode 100644 news/output_file_handling.rst diff --git a/news/986_input_validation.rst b/news/986_input_validation.rst index adeed8591..677e0ae9f 100644 --- a/news/986_input_validation.rst +++ b/news/986_input_validation.rst @@ -4,7 +4,7 @@ **Changed:** -* ``openfe quickrun`` now fails up-front when the user-supplied output file (``-o``) already exists or has a non-existent parent directory. +* **Deprecated:** @@ -16,7 +16,7 @@ **Fixed:** -* +* ``openfe quickrun`` now creates the parent directory as-needed for user-defined output json paths (``-o``). **Security:** diff --git a/news/output_file_handling.rst b/news/output_file_handling.rst new file mode 100644 index 000000000..2eb26e3b0 --- /dev/null +++ b/news/output_file_handling.rst @@ -0,0 +1,23 @@ +**Added:** + +* + +**Changed:** + +* ``openfe quickrun`` now creates the parent directory as-needed for user-defined output json paths (``-o``). + +**Deprecated:** + +* + +**Removed:** + +* + +**Fixed:** + +* + +**Security:** + +* diff --git a/openfecli/commands/quickrun.py b/openfecli/commands/quickrun.py index c7dd6f833..56c124d36 100644 --- a/openfecli/commands/quickrun.py +++ b/openfecli/commands/quickrun.py @@ -6,7 +6,6 @@ import pathlib from openfecli import OFECommandPlugin -from openfecli.parameters.output import validate_outfile from openfecli.utils import write, print_duration, configure_logger @@ -28,14 +27,14 @@ def _format_exception(exception) -> str: type=click.Path(dir_okay=True, file_okay=False, writable=True, path_type=pathlib.Path), help=( - "directory to store files in (defaults to current directory)" + "Directory in which to store files in (defaults to current directory).\ + If the directory does not exist, it will be created at runtime." ), ) @click.option( 'output', '-o', default=None, - type=click.Path(dir_okay=False, file_okay=True, path_type=pathlib.Path), + type=click.Path(dir_okay=False, file_okay=False, path_type=pathlib.Path), help="Filepath at which to create and write the JSON-formatted results.", - callback=validate_outfile, ) @print_duration def quickrun(transformation, work_dir, output): @@ -96,6 +95,12 @@ def quickrun(transformation, work_dir, output): # TODO: change this to `Transformation.load(transformation)` dct = json.load(transformation, cls=JSON_HANDLER.decoder) trans = gufe.Transformation.from_dict(dct) + + if output is None: + output = work_dir / (str(trans.key) + '_results.json') + else: + output.parent.mkdir(exist_ok=True, parents=True) + write("Planning simulations for this edge...") dag = trans.create() write("Starting the simulations for this edge...") @@ -125,9 +130,6 @@ def quickrun(transformation, work_dir, output): } } - if output is None: - output = work_dir / (str(trans.key) + '_results.json') - with open(output, mode='w') as outf: json.dump(out_dict, outf, cls=JSON_HANDLER.encoder) diff --git a/openfecli/parameters/output.py b/openfecli/parameters/output.py index c943537df..440c9cfea 100644 --- a/openfecli/parameters/output.py +++ b/openfecli/parameters/output.py @@ -8,21 +8,6 @@ def get_file_and_extension(user_input, context): ext = file.name.split('.')[-1] if file else None return file, ext - -def ensure_file_does_not_exist(value): - # TODO: I believe we can replace this with click.option(file_okay=False) - if value and value.exists(): - raise click.BadParameter(f"File '{value}' already exists.") - -def ensure_parent_dir_exists(value): - if value and not value.parent.is_dir(): - raise click.BadParameter(f"Cannot write to {value}, parent directory does not exist.") - -def validate_outfile(ctx, param, value): - ensure_file_does_not_exist(value) - ensure_parent_dir_exists(value) - return value - OUTPUT_FILE_AND_EXT = Option( "-o", "--output", help="output file", diff --git a/openfecli/tests/commands/test_quickrun.py b/openfecli/tests/commands/test_quickrun.py index b82d281e5..59a171572 100644 --- a/openfecli/tests/commands/test_quickrun.py +++ b/openfecli/tests/commands/test_quickrun.py @@ -47,21 +47,32 @@ def test_quickrun(extra_args, json_file): def test_quickrun_output_file_exists(json_file): + """Fail if the output file already exists.""" runner = CliRunner() with runner.isolated_filesystem(): pathlib.Path('foo.json').touch() result = runner.invoke(quickrun, [json_file, '-o', 'foo.json']) assert result.exit_code == 2 # usage error - assert "File 'foo.json' already exists." in result.output + assert "is a file." in result.output def test_quickrun_output_file_in_nonexistent_directory(json_file): - """Should catch invalid filepaths up front.""" + """Should create the parent directory for output file if it doesn't exist.""" runner = CliRunner() - outfile = "not_dir/foo.json" - result = runner.invoke(quickrun, [json_file, '-o', outfile]) - assert result.exit_code == 2 - assert "Cannot write" in result.output - + with runner.isolated_filesystem(): + outfile = pathlib.Path("not_dir/foo.json") + result = runner.invoke(quickrun, [json_file, '-o', outfile]) + assert result.exit_code == 0 + assert outfile.parent.is_dir() + +def test_quickrun_dir_created_at_runtime(json_file): + """It should be valid to have a new directory created by the -d flag.""" + runner = CliRunner() + with runner.isolated_filesystem(): + outdir = "not_dir" + outfile = outdir+"foo.json" + result = runner.invoke(quickrun, [json_file, '-d', outdir, '-o', outfile]) + assert result.exit_code == 0 + def test_quickrun_unit_error(): with resources.files('openfecli.tests.data') as d: json_file = str(d / 'bad_transformation.json')