Restructure morphpy for easier maintainence (#235)

Author: Sparks29032

news/restructure_morphpy.rst

@@ -0,0 +1,23 @@
+**Added:**
+
+* <news item>
+
+**Changed:**
+
+* For diffpy.morph developers: both morphpy functions now have shared code in a separate function for easier maintenance.
+
+**Deprecated:**
+
+* <news item>
+
+**Removed:**
+
+* <news item>
+
+**Fixed:**
+
+* <news item>
+
+**Security:**
+
+* <news item>

src/diffpy/morph/morphpy.py

@@ -19,6 +19,57 @@ def get_args(parser, params, kwargs):
     return opts, pargs
 
 
+def __get_morph_opts__(parser, scale, stretch, smear, plot, **kwargs):
+    # Check for Python-specific options
+    python_morphs = ["funcy"]
+    pymorphs = {}
+    for pmorph in python_morphs:
+        if pmorph in kwargs:
+            pmorph_value = kwargs.pop(pmorph)
+            pymorphs.update({pmorph: pmorph_value})
+
+    # Special handling of parameters with dashes
+    kwargs_copy = kwargs.copy()
+    kwargs = {}
+    for key in kwargs_copy.keys():
+        new_key = key
+        if "_" in key:
+            new_key = key.replace("_", "-")
+        kwargs.update({new_key: kwargs_copy[key]})
+
+    # Special handling of store_true and store_false parameters
+    opts_storing_values = [
+        "verbose",
+        "pearson",
+        "addpearson",
+        "apply",
+        "reverse",
+    ]
+    opts_to_ignore = ["multiple-morphs", "multiple-targets"]
+    for opt in opts_storing_values:
+        if opt in kwargs:
+            # Remove if user sets false in params
+            if not kwargs[opt]:
+                kwargs.pop(opt)
+    for opt in opts_to_ignore:
+        if opt in kwargs:
+            kwargs.pop(opt)
+
+    # Wrap the CLI
+    params = {
+        "scale": scale,
+        "stretch": stretch,
+        "smear": smear,
+        "noplot": True if not plot else None,
+    }
+    opts, _ = get_args(parser, params, kwargs)
+
+    if not len(pymorphs) > 0:
+        pymorphs = None
+
+    return opts, pymorphs
+
+
 # Take in file names as input.
 def morph(
     morph_file,
@@ -58,38 +109,12 @@ def morph(
         Function after morph where morph_table[:,0] is the abscissa and
         morph_table[:,1] is the ordinate.
     """
-
-    # Check for Python-specific morphs
-    python_morphs = ["funcy"]
-    pymorphs = {}
-    for pmorph in python_morphs:
-        if pmorph in kwargs:
-            pmorph_value = kwargs.pop(pmorph)
-            pymorphs.update({pmorph: pmorph_value})
-
-    # Special handling of parameters with dashes
-    kwargs_copy = kwargs.copy()
-    kwargs = {}
-    for key in kwargs_copy.keys():
-        new_key = key
-        if "_" in key:
-            new_key = key.replace("_", "-")
-        kwargs.update({new_key: kwargs_copy[key]})
-
-    # Wrap the CLI
-    parser = create_option_parser()
-    params = {
-        "scale": scale,
-        "stretch": stretch,
-        "smear": smear,
-        "noplot": True if not plot else None,
-    }
-    opts, _ = get_args(parser, params, kwargs)
-
     pargs = [morph_file, target_file]
+    parser = create_option_parser()
+    opts, pymorphs = __get_morph_opts__(
+        parser, scale, stretch, smear, plot, **kwargs
+    )
 
-    if not len(pymorphs) > 0:
-        pymorphs = None
     return single_morph(
         parser,
         opts,
@@ -139,36 +164,18 @@ def morph_arrays(
         Function after morph where morph_table[:,0] is the abscissa and
         morph_table[:,1] is the ordinate.
     """
-    # Check for Python-specific morphs
-    python_morphs = ["funcy"]
-    pymorphs = {}
-    for pmorph in python_morphs:
-        if pmorph in kwargs:
-            pmorph_value = kwargs.pop(pmorph)
-            pymorphs.update({pmorph: pmorph_value})
-
-    # Wrap the CLI
-    parser = create_option_parser()
-    params = {
-        "scale": scale,
-        "stretch": stretch,
-        "smear": smear,
-        "noplot": True if not plot else None,
-    }
-    opts, _ = get_args(parser, params, kwargs)
-
     morph_table = np.array(morph_table)
     target_table = np.array(target_table)
-
     x_morph = morph_table[:, 0]
     y_morph = morph_table[:, 1]
     x_target = target_table[:, 0]
     y_target = target_table[:, 1]
-
     pargs = ["Morph", "Target", x_morph, y_morph, x_target, y_target]
+    parser = create_option_parser()
+    opts, pymorphs = __get_morph_opts__(
+        parser, scale, stretch, smear, plot, **kwargs
+    )
 
-    if not len(pymorphs) > 0:
-        pymorphs = None
     return single_morph(
         parser,
         opts,

tests/test_morphpy.py

@@ -6,7 +6,7 @@
 import pytest
 
 from diffpy.morph.morphapp import create_option_parser, single_morph
-from diffpy.morph.morphpy import morph, morph_arrays
+from diffpy.morph.morphpy import __get_morph_opts__, morph, morph_arrays
 from diffpy.morph.tools import getRw
 
 thisfile = locals().get("__file__", "file.py")
@@ -68,6 +68,52 @@ def setup_morph(self):
                 )
         return
 
+    def test_morph_opts(self, setup_morph):
+        kwargs = {
+            "verbose": False,
+            "pearson": False,
+            "addpearson": False,
+            "apply": False,
+            "reverse": False,
+            "multiple_morphs": False,
+            "multiple_targets": False,
+        }
+        kwargs_copy = kwargs.copy()
+        opts, _ = __get_morph_opts__(
+            self.parser, scale=1, stretch=0, smear=0, plot=False, **kwargs_copy
+        )
+        # Special set true/false operations should be removed
+        # when their input value is False
+        for opt in kwargs:
+            if opt == "apply":
+                assert getattr(opts, "refine")
+            else:
+                assert getattr(opts, opt) is None or not getattr(opts, opt)
+
+        kwargs = {
+            "verbose": True,
+            "pearson": True,
+            "addpearson": True,
+            "apply": True,
+            "reverse": True,
+            "multiple_morphs": True,
+            "multiple_targets": True,
+        }
+        kwargs_copy = kwargs.copy()
+        opts, _ = __get_morph_opts__(
+            self.parser, scale=1, stretch=0, smear=0, plot=False, **kwargs_copy
+        )
+        for opt in kwargs:
+            if opt == "apply":
+                assert not getattr(opts, "refine")
+            # These options are not enabled in morphpy
+            elif opt == "multiple_morphs" or opt == "multiple_targets":
+                assert getattr(opts, opt) is None or not getattr(opts, opt)
+            # Special set true/false operations should NOT be removed
+            # when their input value is True
+            else:
+                assert getattr(opts, opt)
+
     def test_morph(self, setup_morph):
         morph_results = {}
         morph_file = self.testfiles[0]