Support outdir created at runtime (#1072)

* adding test

* create directories

* fixing a word

* update news

* update news

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.
+* <news item>
 
 **Deprecated:**
 
@@ -16,7 +16,7 @@
 
 **Fixed:**
 
-* <news item>
+* ``openfe quickrun`` now creates the parent directory as-needed for user-defined output json paths (``-o``).
 
 **Security:**
 

news/output_file_handling.rst

@@ -0,0 +1,23 @@
+**Added:**
+
+* <news item>
+
+**Changed:**
+
+* ``openfe quickrun`` now creates the parent directory as-needed for user-defined output json paths (``-o``).
+
+**Deprecated:**
+
+* <news item>
+
+**Removed:**
+
+* <news item>
+
+**Fixed:**
+
+* <news item>
+
+**Security:**
+
+* <news item>

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)
 

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",

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')