Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions news/muD-options.rst
Original file line number Diff line number Diff line change
@@ -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>
43 changes: 28 additions & 15 deletions src/diffpy/labpdfproc/labpdfprocapp.py
Original file line number Diff line number Diff line change
Expand Up @@ -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": (
Expand Down Expand Up @@ -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:
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

grouped the related mu*D arguments

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()
Expand All @@ -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()
Expand Down
39 changes: 29 additions & 10 deletions src/diffpy/labpdfproc/tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
----------
Expand All @@ -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)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made the z-scan part a separate private function

return args


Expand Down Expand Up @@ -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

Expand Down
89 changes: 55 additions & 34 deletions tests/test_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Manually entered muD following the new convention ("2.5" to "--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)
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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"]
Expand Down Expand Up @@ -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)
Expand All @@ -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(
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new tests

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


Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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",
Expand All @@ -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 == {
Expand All @@ -523,8 +543,9 @@ def test_load_metadata(mocker, user_filesystem):
),
)
cli_inputs = [
"2.5",
".",
"--mud",
"2.5",
"--anode-type",
"Mo",
"--user-metadata",
Expand Down
Loading