Merge pull request diffpy#168 from yucongalicechen/muD

feat: make muD optional and have different options for specifying muD

Author: sbillinge

news/muD-options.rst

@@ -0,0 +1,23 @@
+**Added:**
+
+* <news item>
+
+**Changed:**
+
+* Made muD an optional argument and provided different options (manually entry / z-scan file path) for users to specify muD
+
+**Deprecated:**
+
+* <news item>
+
+**Removed:**
+
+* <news item>
+
+**Fixed:**
+
+* <news item>
+
+**Security:**
+
+* <news item>

src/diffpy/labpdfproc/labpdfprocapp.py

@@ -13,13 +13,8 @@
 from diffpy.utils.parsers.loaddata import loadData
 
 
-def define_arguments():
+def _define_arguments():
     args = [
-        {
-            "name": ["mud"],
-            "help": "Value of mu*D for your sample. Required.",
-            "type": float,
-        },
         {
             "name": ["input"],
             "help": (
@@ -150,20 +145,37 @@ def define_arguments():
             ),
             "default": None,
         },
-        {
-            "name": ["-z", "--z-scan-file"],
-            "help": "Path to the z-scan file to be loaded "
-            "to determine the mu*D value.",
-            "default": None,
-            "widget": "FileChooser",
-        },
     ]
     return args
 
 
+def _add_mud_selection_group(p, is_gui=False):
+    """Current Options:
+    1. Manually enter muD (`--mud`).
+    2. Estimate muD from a z-scan file (`-z` or `--z-scan-file`).
+    """
+    g = p.add_argument_group("Options for setting mu*D value (Required)")
+    g = g.add_mutually_exclusive_group(required=True)
+    g.add_argument(
+        "--mud",
+        type=float,
+        help="Enter the mu*D value manually.",
+        **({"widget": "DecimalField"} if is_gui else {}),
+    )
+    g.add_argument(
+        "-z",
+        "--z-scan-file",
+        help="Provide the path to the z-scan file to be loaded "
+        "to determine the mu*D value.",
+        **({"widget": "FileChooser"} if is_gui else {}),
+    )
+    return p
+
+
 def get_args(override_cli_inputs=None):
     p = ArgumentParser()
-    for arg in define_arguments():
+    p = _add_mud_selection_group(p, is_gui=False)
+    for arg in _define_arguments():
         kwargs = {
             key: value
             for key, value in arg.items()
@@ -177,7 +189,8 @@ def get_args(override_cli_inputs=None):
 @Gooey(required_cols=1, optional_cols=1, program_name="Labpdfproc GUI")
 def gooey_parser():
     p = GooeyParser()
-    for arg in define_arguments():
+    p = _add_mud_selection_group(p, is_gui=True)
+    for arg in _define_arguments():
         kwargs = {key: value for key, value in arg.items() if key != "name"}
         p.add_argument(*arg["name"], **kwargs)
     args = p.parse_args()

src/diffpy/labpdfproc/tools.py

@@ -251,8 +251,34 @@ def set_xtype(args):
     return args
 
 
+def _estimate_mud_from_zscan(args):
+    """Compute mu*D based on the given z-scan file.
+
+    Parameters
+    ----------
+    args : argparse.Namespace
+        The arguments from the parser.
+
+    Returns
+    -------
+    args : argparse.Namespace
+        The updated arguments with mu*D.
+    """
+    filepath = Path(args.z_scan_file).resolve()
+    if not filepath.is_file():
+        raise FileNotFoundError(
+            f"Cannot find {args.z_scan_file}. "
+            f"Please specify a valid file path."
+        )
+    args.z_scan_file = str(filepath)
+    args.mud = compute_mud(filepath)
+    return args
+
+
 def set_mud(args):
-    """Compute mu*D based on the given z-scan file, if provided.
+    """Compute and set mu*D based on different options.
+    Current options include manually entering a value,
+    or estimating from a z-scan file.
 
     Parameters
     ----------
@@ -265,14 +291,7 @@ def set_mud(args):
         The updated arguments with mu*D.
     """
     if args.z_scan_file:
-        filepath = Path(args.z_scan_file).resolve()
-        if not filepath.is_file():
-            raise FileNotFoundError(
-                f"Cannot find {args.z_scan_file}. "
-                f"Please specify a valid file path."
-            )
-        args.z_scan_file = str(filepath)
-        args.mud = compute_mud(filepath)
+        return _estimate_mud_from_zscan(args)
     return args
 
 
@@ -390,13 +409,13 @@ def preprocessing_args(args):
     args : argparse.Namespace
         The updated argparse Namespace with arguments preprocessed.
     """
+    args = set_mud(args)
     args = load_package_info(args)
     args = load_user_info(args)
     args = set_input_lists(args)
     args = set_output_directory(args)
     args = set_wavelength(args)
     args = set_xtype(args)
-    args = set_mud(args)
     args = load_user_metadata(args)
     return args
 

tests/test_tools.py

@@ -113,7 +113,7 @@ def test_set_input_lists(inputs, expected, user_filesystem):
         base_dir.resolve() / expected_path for expected_path in expected
     ]
 
-    cli_inputs = ["2.5"] + inputs
+    cli_inputs = inputs + ["--mud", "2.5"]
     actual_args = get_args(cli_inputs)
     actual_args = set_input_lists(actual_args)
     assert sorted(actual_args.input_paths) == sorted(expected_paths)
@@ -159,7 +159,7 @@ def test_set_input_lists(inputs, expected, user_filesystem):
 def test_set_input_files_bad(inputs, expected_error_msg, user_filesystem):
     base_dir = Path(user_filesystem)
     os.chdir(base_dir)
-    cli_inputs = ["2.5"] + inputs
+    cli_inputs = inputs + ["--mud", "2.5"]
     actual_args = get_args(cli_inputs)
     with pytest.raises(FileNotFoundError, match=re.escape(expected_error_msg)):
         actual_args = set_input_lists(actual_args)
@@ -177,7 +177,7 @@ def test_set_input_files_bad(inputs, expected_error_msg, user_filesystem):
 def test_set_output_directory(inputs, expected, user_filesystem):
     os.chdir(user_filesystem)
     expected_output_directory = Path(user_filesystem) / expected[0]
-    cli_inputs = ["2.5", "data.xy"] + inputs
+    cli_inputs = ["data.xy", "--mud", "2.5"] + inputs
     actual_args = get_args(cli_inputs)
     actual_args = set_output_directory(actual_args)
     assert actual_args.output_directory == expected_output_directory
@@ -187,7 +187,13 @@ def test_set_output_directory(inputs, expected, user_filesystem):
 
 def test_set_output_directory_bad(user_filesystem):
     os.chdir(user_filesystem)
-    cli_inputs = ["2.5", "data.xy", "--output-directory", "good_data.chi"]
+    cli_inputs = [
+        "data.xy",
+        "--mud",
+        "2.5",
+        "--output-directory",
+        "good_data.chi",
+    ]
     actual_args = get_args(cli_inputs)
     with pytest.raises(FileExistsError):
         actual_args = set_output_directory(actual_args)
@@ -247,7 +253,7 @@ def test_set_output_directory_bad(user_filesystem):
     ],
 )
 def test_set_wavelength(inputs, expected):
-    cli_inputs = ["2.5", "data.xy"] + inputs
+    cli_inputs = ["data.xy", "--mud", "2.5"] + inputs
     actual_args = get_args(cli_inputs)
     actual_args = set_wavelength(actual_args)
     assert actual_args.wavelength == expected["wavelength"]
@@ -286,7 +292,7 @@ def test_set_wavelength(inputs, expected):
     ],
 )
 def test_set_wavelength_bad(inputs, expected_error_msg):
-    cli_inputs = ["2.5", "data.xy"] + inputs
+    cli_inputs = ["data.xy", "--mud", "2.5"] + inputs
     actual_args = get_args(cli_inputs)
     with pytest.raises(ValueError, match=re.escape(expected_error_msg)):
         actual_args = set_wavelength(actual_args)
@@ -302,50 +308,63 @@ def test_set_wavelength_bad(inputs, expected_error_msg):
     ],
 )
 def test_set_xtype(inputs, expected_xtype):
-    cli_inputs = ["2.5", "data.xy"] + inputs
+    cli_inputs = ["data.xy", "--mud", "2.5"] + inputs
     actual_args = get_args(cli_inputs)
     actual_args = set_xtype(actual_args)
     assert actual_args.xtype == expected_xtype
 
 
 def test_set_xtype_bad():
-    cli_inputs = ["2.5", "data.xy", "--xtype", "invalid"]
+    cli_inputs = ["data.xy", "--mud", "2.5", "--xtype", "invalid"]
     actual_args = get_args(cli_inputs)
     with pytest.raises(
         ValueError,
         match=re.escape(
-            f"Unknown xtype: invalid. " f"Allowed xtypes are {*XQUANTITIES, }."
+            f"Unknown xtype: invalid. Allowed xtypes are {*XQUANTITIES, }."
         ),
     ):
         actual_args = set_xtype(actual_args)
 
 
-def test_set_mud(user_filesystem):
-    cli_inputs = ["2.5", "data.xy"]
-    actual_args = get_args(cli_inputs)
-    actual_args = set_mud(actual_args)
-    assert actual_args.mud == pytest.approx(2.5, rel=1e-4, abs=0.1)
-    assert actual_args.z_scan_file is None
-
+@pytest.mark.parametrize(
+    "inputs, expected_mud",
+    [
+        # C1: user enters muD manually, expect to return the same value
+        (["--mud", "2.5"], 2.5),
+        # C2: user provides a z-scan file, expect to estimate through the file
+        (["--z-scan-file", "test_dir/testfile.xy"], 3),
+    ],
+)
+def test_set_mud(user_filesystem, inputs, expected_mud):
     cwd = Path(user_filesystem)
-    test_dir = cwd / "test_dir"
     os.chdir(cwd)
-    inputs = ["--z-scan-file", "test_dir/testfile.xy"]
-    expected = [3, str(test_dir / "testfile.xy")]
-    cli_inputs = ["2.5", "data.xy"] + inputs
+    cli_inputs = ["data.xy"] + inputs
     actual_args = get_args(cli_inputs)
     actual_args = set_mud(actual_args)
-    assert actual_args.mud == pytest.approx(expected[0], rel=1e-4, abs=0.1)
-    assert actual_args.z_scan_file == expected[1]
+    assert actual_args.mud == pytest.approx(expected_mud, rel=1e-4, abs=0.1)
 
 
-def test_set_mud_bad():
-    cli_inputs = ["2.5", "data.xy", "--z-scan-file", "invalid file"]
+@pytest.mark.parametrize(
+    "inputs, expected",
+    [
+        # C1: user provides an invalid z-scan file,
+        # expect FileNotFoundError and message to specify a valid file path
+        (
+            ["--z-scan-file", "invalid file"],
+            [
+                FileNotFoundError,
+                "Cannot find invalid file. Please specify a valid file path.",
+            ],
+        ),
+    ],
+)
+def test_set_mud_bad(user_filesystem, inputs, expected):
+    expected_error, expected_error_msg = expected
+    cwd = Path(user_filesystem)
+    os.chdir(cwd)
+    cli_inputs = ["data.xy"] + inputs
     actual_args = get_args(cli_inputs)
-    with pytest.raises(
-        FileNotFoundError,
-        match="Cannot find invalid file. " "Please specify a valid file path.",
-    ):
+    with pytest.raises(expected_error, match=expected_error_msg):
         actual_args = set_mud(actual_args)
 
 
@@ -370,12 +389,12 @@ def test_set_mud_bad():
     ],
 )
 def test_load_user_metadata(inputs, expected):
-    expected_args = get_args(["2.5", "data.xy"])
+    expected_args = get_args(["data.xy", "--mud", "2.5"])
     for expected_pair in expected:
         setattr(expected_args, expected_pair[0], expected_pair[1])
     delattr(expected_args, "user_metadata")
 
-    cli_inputs = ["2.5", "data.xy"] + inputs
+    cli_inputs = ["data.xy", "--mud", "2.5"] + inputs
     actual_args = get_args(cli_inputs)
     actual_args = load_user_metadata(actual_args)
     assert actual_args == expected_args
@@ -411,7 +430,7 @@ def test_load_user_metadata(inputs, expected):
     ],
 )
 def test_load_user_metadata_bad(inputs, expected_error_msg):
-    cli_inputs = ["2.5", "data.xy"] + inputs
+    cli_inputs = ["data.xy", "--mud", "2.5"] + inputs
     actual_args = get_args(cli_inputs)
     with pytest.raises(ValueError, match=re.escape(expected_error_msg)):
         actual_args = load_user_metadata(actual_args)
@@ -474,8 +493,9 @@ def test_load_user_info(monkeypatch, inputs, expected, user_filesystem):
     os.chdir(cwd)
 
     cli_inputs = [
-        "2.5",
         "data.xy",
+        "--mud",
+        "2.5",
         "--username",
         inputs["username"],
         "--email",
@@ -497,7 +517,7 @@ def test_load_package_info(mocker):
             "3.3.0" if package_name == "diffpy.utils" else "1.2.3"
         ),
     )
-    cli_inputs = ["2.5", "data.xy"]
+    cli_inputs = ["data.xy", "--mud", "2.5"]
     actual_args = get_args(cli_inputs)
     actual_args = load_package_info(actual_args)
     assert actual_args.package_info == {
@@ -523,8 +543,9 @@ def test_load_metadata(mocker, user_filesystem):
         ),
     )
     cli_inputs = [
-        "2.5",
         ".",
+        "--mud",
+        "2.5",
         "--anode-type",
         "Mo",
         "--user-metadata",