Repository: microsoft/pybryt

Issue:
Unify CLI argument language
There's some small disconnects in the naming of CLI arguments, e.g. the interchangeability of `dest` and `output`. These should be fixed. This would also allow the inclusion of the output argument for student implementations in `pybryt execute`.

Discussion:

Patch:
diff --git a/pybryt/cli.py b/pybryt/cli.py
index 2920198..cbcbabe 100644
--- a/pybryt/cli.py
+++ b/pybryt/cli.py
@@ -37,17 +37,15 @@ def compile_reference(src, dest, name):
 
 
 @click_cli.command()
-@click.option("-n", "--name", default=None, type=click.STRING, 
-              help="Optional name for the reference implementation")
-@click.option("--output-nb", default=None, type=click.Path(dir_okay=False), 
+@click.option("--output", default=None, type=click.Path(dir_okay=False), 
               help="Path at which to write the output notebook from executing the student submission")
-@click.option("-o", "--output", default=None, type=click.Path(dir_okay=False), 
+@click.option("-d", "--dest", default=None, type=click.Path(dir_okay=False), 
               help="Path at which to write the results of the check")
 @click.option("-t", "--type", "output_type", default="pickle", show_default=True, 
               type=click.Choice(["pickle", "json", "report"]), help="Type of output to write")
 @click.argument("ref", type=click.Path(exists=True, dir_okay=False))
 @click.argument("stu", type=click.Path(exists=True, dir_okay=False))
-def check(ref, stu, name, output_nb, output, output_type):
+def check(ref, stu, output, dest, output_type):
     """
     Run a student submission against a reference implementation.
 
@@ -65,11 +63,11 @@ def check(ref, stu, name, output_nb, output, output_type):
         except:
             raise RuntimeError(f"Could not load the reference implementation {ref}")
 
-    if output is None:
-        output = get_stem(stu) + "_results" + (".pkl", ".json")[output_type == "json"]
+    if dest is None:
+        dest = get_stem(stu) + "_results" + (".pkl", ".json")[output_type == "json"]
 
     if os.path.splitext(stu)[1] == ".ipynb":
-        stu = StudentImplementation(stu, output=output_nb)
+        stu = StudentImplementation(stu, output=output)
     else:
         try:
             stu = StudentImplementation.load(stu)
@@ -80,16 +78,16 @@ def check(ref, stu, name, output_nb, output, output_type):
 
     if output_type == "pickle":
         if isinstance(res, list):
-            with open(output, "wb+") as f:
+            with open(dest, "wb+") as f:
                 dill.dump(res, f)
         else:
-            res.dump(output)
+            res.dump(dest)
     elif output_type == "json":
         if isinstance(res, list):
             d = [r.to_dict() for r in res]
         else:
             d = res.to_dict()
-        with open(output, "w+") as f:
+        with open(dest, "w+") as f:
             json.dump(d, f, indent=2)
     elif output_type == "report":
         report = generate_report(res)
@@ -99,17 +97,17 @@ def check(ref, stu, name, output_nb, output, output_type):
 @click_cli.command()
 @click.option("-p", "--parallel", is_flag=True, default=False, 
               help="Execute notebooks in parallel using the multiprocessing library")
-@click.option("-o", "--output", default=None, type=click.Path(), 
+@click.option("-d", "--dest", default=None, type=click.Path(), 
               help="Path at which to write the pickled student implementation")
 @click.option("--timeout", default=1200, type=click.INT, 
               help="Timeout for notebook execution in seconds")
 @click.argument("subm", nargs=-1, type=click.Path(exists=True, dir_okay=False))
-def execute(subm, parallel, output, timeout):
+def execute(subm, parallel, dest, timeout):
     """
     Execute student submissions to generate memory footprints.
 
     Executes the student submission(s) SUBM at writes the pickled objects to some output file. If
-    OUTPUT is unspecified, this defaults to "./{SUBM.stem}.pkl" (e.g. for SUBM 
+    DEST is unspecified, this defaults to "./{SUBM.stem}.pkl" (e.g. for SUBM 
     "submissions/subm01.ipynb", this is "./subm01.pkl").
     """
     if len(subm) == 0:
@@ -117,23 +115,23 @@ def execute(subm, parallel, output, timeout):
 
     stus = generate_student_impls(subm, parallel=parallel, timeout=timeout)
 
-    if output is None:
-            output = "./"
+    if dest is None:
+            dest = "./"
 
     if len(subm) == 1:
-        if os.path.isdir(output):
+        if os.path.isdir(dest):
             stem = get_stem(subm[0])
-            output = os.path.join(output, stem + ".pkl")
+            output = os.path.join(dest, stem + ".pkl")
 
         stus[0].dump(output)
 
     else:
-        if not os.path.isdir(output):
-            raise ValueError(f"Output directory {output} does not exist or is not a directory")
+        if not os.path.isdir(dest):
+            raise ValueError(f"Destination directory {dest} does not exist or is not a directory")
 
         for s, stu in zip(subm, stus):
             stem = get_stem(s)
-            path = os.path.join(output, stem + ".pkl")
+            path = os.path.join(dest, stem + ".pkl")
             stu.dump(path)
 
 


Test patch:
diff --git a/tests/test_cli.py b/tests/test_cli.py
index 69528d8..3db1cba 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -143,10 +143,10 @@ def test_execute():
         mocked_generate.assert_called_with(fns, parallel=False, timeout=100)
 
         # check for error on nonexistance output dir
-        result = runner.invoke(click_cli, ["execute", *fns, "-o", "/some/fake/path"])
+        result = runner.invoke(click_cli, ["execute", *fns, "-d", "/some/fake/path"])
         assert result.exit_code == 1
         assert isinstance(result.exception, ValueError)
-        assert result.exception.args[0] == "Output directory /some/fake/path does not exist or is not a directory"
+        assert result.exception.args[0] == "Destination directory /some/fake/path does not exist or is not a directory"
 
     # check other errors
     result = runner.invoke(click_cli, ["execute"])