Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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/squeeze_lims.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
**Added:**

* Error thrown when squeeze morph given improper inputs.

**Changed:**

* Squeeze morph now removes duplicate/repeated and trailing commas before parsing.

**Deprecated:**

* <news item>

**Removed:**

* <news item>

**Fixed:**

* <news item>

**Security:**

* <news item>
22 changes: 14 additions & 8 deletions src/diffpy/morph/morphapp.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ def custom_error(self, msg):
"a0+a1*x+a2*x^2+...a_n*x^n."
"n is dependent on the number of values in the "
"user-inputted comma-separated list. "
"Repeated and trailing commas are removed before parsing."
"When this option is enabled, --hshift is disabled. "
"When n>1, --stretch is disabled. "
"See online documentation for more information."
Expand Down Expand Up @@ -522,6 +523,7 @@ def single_morph(

# Squeeze
squeeze_poly_deg = -1
squeeze_dict_in = {}
if opts.squeeze is not None:
# Handles both list and csv input
if (
Expand All @@ -537,12 +539,19 @@ def single_morph(
):
opts.squeeze = opts.squeeze[1:-1]
squeeze_coeffs = opts.squeeze.strip().split(",")
squeeze_dict_in = {}
for idx, coeff in enumerate(squeeze_coeffs):
squeeze_dict_in.update({f"a{idx}": float(coeff)})
squeeze_poly_deg = len(squeeze_coeffs) - 1
idx = 0
for _, coeff in enumerate(squeeze_coeffs):
if coeff.strip() != "":
try:
squeeze_dict_in.update({f"a{idx}": float(coeff)})
idx += 1
except ValueError:
parser.error(f"{coeff} could not be converted to float.")
squeeze_poly_deg = len(squeeze_dict_in.keys())
chain.append(morphs.MorphSqueeze())
config["squeeze"] = squeeze_dict_in
# config["extrap_index_low"] = None
# config["extrap_index_high"] = None
refpars.append("squeeze")
# Scale
if opts.scale is not None:
Expand Down Expand Up @@ -679,10 +688,7 @@ def single_morph(
morph_inputs.update({"hshift": hshift_in, "vshift": vshift_in})
# More complex input morph parameters are only displayed conditionally
if opts.squeeze is not None:
squeeze_coeffs = opts.squeeze.strip().split(",")
squeeze_dict = {}
for idx, coeff in enumerate(squeeze_coeffs):
squeeze_dict.update({f"a{idx}": float(coeff)})
squeeze_dict = squeeze_dict_in.copy()
for idx, _ in enumerate(squeeze_dict):
morph_inputs.update({f"squeeze a{idx}": squeeze_dict[f"a{idx}"]})
if pymorphs is not None:
Expand Down
12 changes: 12 additions & 0 deletions tests/test_morphapp.py
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,18 @@ def test_parser_systemexits(self, setup_parser):
with pytest.raises(SystemExit):
multiple_targets(self.parser, opts, pargs, stdout_flag=False)

# Pass a non-float list to squeeze
(opts, pargs) = self.parser.parse_args(
[
f"{nickel_PDF}",
f"{nickel_PDF}",
"--squeeze",
"1,a,0",
]
)
with pytest.raises(SystemExit):
Copy link
Contributor

Choose a reason for hiding this comment

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

why are we using sys.exit and not a proper exception? In general, this is to be avoided as it bypasses all the nice exception handling you get for free in Python?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a test for the CLI, and the intended behavior is that once we receive a ValueError from the 'a' character, the parser raises an error, and the CLI program exits with value 1.

Copy link
Collaborator Author

@Sparks29032 Sparks29032 Jul 1, 2025

Choose a reason for hiding this comment

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

The line parser.error(f"{coeff} could not be converted to float.") (line 549 on 'morphapp.py') is calling the in-built option parser function to handle the error once we detect a ValueError on the Python side.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm, but we should ensure that the test is capturing this particular error, and not necessarily system exiting elsewhere. Let me put a new commit in a bit.

single_morph(self.parser, opts, pargs, stdout_flag=False)

def test_morphsequence(self, setup_morphsequence):
# Parse arguments sorting by field
(opts, pargs) = self.parser.parse_args(
Expand Down
4 changes: 3 additions & 1 deletion tests/test_morphio.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,9 @@ def test_morphsqueeze_outputs(self, setup, tmp_path):
"--scale",
"2",
"--squeeze",
"0,-0.001,-0.0001,0.0001",
# Ignore duplicate commas and trailing commas
# Handle spaces and non-spaces
"0,, ,-0.001, -0.0001,0.0001,",
"--stretch",
"1",
"--hshift",
Expand Down
Loading