Edge cases and error handling for the squeeze morph (#227)

* Edge cases and error handling

* News

* Tests now check message details

Author: Sparks29032

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:**
+
+* <news item>
+
+**Removed:**
+
+* <news item>
+
+**Fixed:**
+
+* <news item>
+
+**Security:**
+
+* <news item>

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:

tests/test_morphapp.py

@@ -105,27 +105,43 @@ 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(
             [f"{nickel_PDF}", f"{nickel_PDF}", "--rmin", "10", "--rmax", "1"]
         )
         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,15 +172,21 @@ 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(
             [f"{nickel_PDF}", f"{testsequence_dir}", "--sort-by", "fake_field"]
         )
         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,26 @@ 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(
+            [
+                f"{nickel_PDF}",
+                f"{nickel_PDF}",
+                "--squeeze",
+                "1,a,0",
+            ]
+        )
+        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

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