From 2b54742b5b8f02abe2e426af63f992385d5e27d8 Mon Sep 17 00:00:00 2001 From: Sparks29032 Date: Mon, 30 Jun 2025 14:13:24 -0500 Subject: [PATCH 1/3] Edge cases and error handling --- src/diffpy/morph/morphapp.py | 22 ++++++++++++++-------- tests/test_morphapp.py | 12 ++++++++++++ tests/test_morphio.py | 4 +++- 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/src/diffpy/morph/morphapp.py b/src/diffpy/morph/morphapp.py index 2dd6c623..70737bf2 100755 --- a/src/diffpy/morph/morphapp.py +++ b/src/diffpy/morph/morphapp.py @@ -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." @@ -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 ( @@ -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: @@ -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: diff --git a/tests/test_morphapp.py b/tests/test_morphapp.py index 9abd2463..73dd774b 100644 --- a/tests/test_morphapp.py +++ b/tests/test_morphapp.py @@ -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): + 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( diff --git a/tests/test_morphio.py b/tests/test_morphio.py index 37f7dcd1..7311113a 100644 --- a/tests/test_morphio.py +++ b/tests/test_morphio.py @@ -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", From 84954e5a1b55f96c664ce769f5922f20a3bf1232 Mon Sep 17 00:00:00 2001 From: Sparks29032 Date: Mon, 30 Jun 2025 14:21:56 -0500 Subject: [PATCH 2/3] News --- news/squeeze_lims.rst | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 news/squeeze_lims.rst diff --git a/news/squeeze_lims.rst b/news/squeeze_lims.rst new file mode 100644 index 00000000..e27b64b0 --- /dev/null +++ b/news/squeeze_lims.rst @@ -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:** + +* + +**Removed:** + +* + +**Fixed:** + +* + +**Security:** + +* From 8503cce9d7f56d751b5b7425fdcadd207637d5f9 Mon Sep 17 00:00:00 2001 From: Sparks29032 Date: Tue, 1 Jul 2025 12:42:18 -0500 Subject: [PATCH 3/3] Tests now check message details --- tests/test_morphapp.py | 40 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/tests/test_morphapp.py b/tests/test_morphapp.py index 73dd774b..379d9f22 100644 --- a/tests/test_morphapp.py +++ b/tests/test_morphapp.py @@ -105,20 +105,34 @@ def test_parser_numerical(self, setup_parser): ) # Check correct value parsed assert len(n_args) == 1 # Check one leftover - def test_parser_systemexits(self, setup_parser): + def test_parser_systemexits(self, capsys, setup_parser): # ###Basic tests for any variety of morphing### # Ensure only two pargs given for morphing (opts, pargs) = self.parser.parse_args(["toofewfiles"]) with pytest.raises(SystemExit): single_morph(self.parser, opts, pargs, stdout_flag=False) + _, err = capsys.readouterr() + assert "You must supply FILE1 and FILE2." in err with pytest.raises(SystemExit): multiple_targets(self.parser, opts, pargs, stdout_flag=False) + _, err = capsys.readouterr() + assert "You must supply FILE and DIRECTORY." in err (opts, pargs) = self.parser.parse_args(["too", "many", "files"]) with pytest.raises(SystemExit): single_morph(self.parser, opts, pargs, stdout_flag=False) + _, err = capsys.readouterr() + assert ( + "Too many arguments. Make sure you only supply FILE1 and FILE2." + in err + ) with pytest.raises(SystemExit): multiple_targets(self.parser, opts, pargs, stdout_flag=False) + _, err = capsys.readouterr() + assert ( + "Too many arguments. You must only supply a FILE and a DIRECTORY." + in err + ) # Make sure rmax greater than rmin (opts, pargs) = self.parser.parse_args( @@ -126,6 +140,8 @@ def test_parser_systemexits(self, setup_parser): ) with pytest.raises(SystemExit): single_morph(self.parser, opts, pargs, stdout_flag=False) + _, err = capsys.readouterr() + assert "rmin must be less than rmax" in err # ###Tests exclusive to multiple morphs### # Make sure we save to a directory that exists @@ -140,8 +156,12 @@ def test_parser_systemexits(self, setup_parser): ) with pytest.raises(SystemExit): single_morph(self.parser, opts, pargs, stdout_flag=False) + _, err = capsys.readouterr() + assert "Unable to save to designated location." in err with pytest.raises(SystemExit): multiple_targets(self.parser, opts, pargs, stdout_flag=False) + _, err = capsys.readouterr() + assert "is not a directory." in err # Ensure first parg is a FILE and second parg is a DIRECTORY (opts, pargs) = self.parser.parse_args( @@ -152,8 +172,12 @@ def test_parser_systemexits(self, setup_parser): (opts, pargs) = self.parser.parse_args( [f"{testsequence_dir}", f"{testsequence_dir}"] ) + _, err = capsys.readouterr() + assert "is not a directory." in err with pytest.raises(SystemExit): multiple_targets(self.parser, opts, pargs, stdout_flag=False) + _, err = capsys.readouterr() + assert "is not a file." in err # Try sorting by non-existing field (opts, pargs) = self.parser.parse_args( @@ -161,6 +185,8 @@ def test_parser_systemexits(self, setup_parser): ) with pytest.raises(SystemExit): multiple_targets(self.parser, opts, pargs, stdout_flag=False) + _, err = capsys.readouterr() + assert "The requested field is missing from a PDF file header." in err (opts, pargs) = self.parser.parse_args( [ f"{nickel_PDF}", @@ -173,6 +199,8 @@ def test_parser_systemexits(self, setup_parser): ) with pytest.raises(SystemExit): multiple_targets(self.parser, opts, pargs, stdout_flag=False) + _, err = capsys.readouterr() + assert "The requested field was not found in the metadata file." in err # Try plotting an unknown parameter (opts, pargs) = self.parser.parse_args( @@ -185,6 +213,8 @@ def test_parser_systemexits(self, setup_parser): ) with pytest.raises(SystemExit): multiple_targets(self.parser, opts, pargs, stdout_flag=False) + _, err = capsys.readouterr() + assert "Cannot find specified plot parameter. No plot shown." in err # Try plotting an unrefined parameter (opts, pargs) = self.parser.parse_args( @@ -197,6 +227,12 @@ def test_parser_systemexits(self, setup_parser): ) with pytest.raises(SystemExit): multiple_targets(self.parser, opts, pargs, stdout_flag=False) + _, err = capsys.readouterr() + assert ( + "The plot parameter is missing values for at " + "least one morph and target pair. " + "No plot shown." in err + ) # Pass a non-float list to squeeze (opts, pargs) = self.parser.parse_args( @@ -209,6 +245,8 @@ def test_parser_systemexits(self, setup_parser): ) with pytest.raises(SystemExit): single_morph(self.parser, opts, pargs, stdout_flag=False) + _, err = capsys.readouterr() + assert "a could not be converted to float." in err def test_morphsequence(self, setup_morphsequence): # Parse arguments sorting by field