Merge pull request #184 from Sparks29032/bugfix

Bug (unintentional feature) fixes

Author: sbillinge

doc/source/api/diffpy.morph.rst

@@ -20,10 +20,10 @@ Subpackages
 Submodules
 ----------
 
-diffpy.morph.pdfplot module
-^^^^^^^^^^^^^^^^^^^^^^^^^^^
+diffpy.morph.plot module
+^^^^^^^^^^^^^^^^^^^^^^^^
 
-.. automodule:: diffpy.morph.pdfplot
+.. automodule:: diffpy.morph.plot
     :members:
     :undoc-members:
     :show-inheritance:

news/bugfix.rst

@@ -0,0 +1,24 @@
+**Added:**
+
+* <news item>
+
+**Changed:**
+
+* Swap colors for morph and target. Morph is now blue and target red.
+
+**Deprecated:**
+
+* <news item>
+
+**Removed:**
+
+* <news item>
+
+**Fixed:**
+
+* Multiple morphs/targets used to break given multiple subdirectories.
+* Fixed the Rw calculation. Previously returned an analogue to chi square.
+
+**Security:**
+
+* <news item>

src/diffpy/morph/morphapp.py

@@ -21,7 +21,7 @@
 import diffpy.morph.morph_helpers as helpers
 import diffpy.morph.morph_io as io
 import diffpy.morph.morphs as morphs
-import diffpy.morph.pdfplot as pdfplot
+import diffpy.morph.plot as plot
 import diffpy.morph.refine as refine
 import diffpy.morph.tools as tools
 from diffpy.morph import __save_morph_as__
@@ -588,22 +588,22 @@ def single_morph(parser, opts, pargs, stdout_flag=True):
         parser.custom_error(save_fail_message)
 
     if opts.plot:
-        pairlist = [chain.xy_morph_out, chain.xy_target_out]
-        labels = [pargs[0], pargs[1]]  # Default is to use file names
+        pairlist = [chain.xy_target_out, chain.xy_morph_out]
+        labels = [pargs[1], pargs[0]]  # Default is to use file names
 
         # If user chooses labels
         if opts.mlabel is not None:
-            labels[0] = opts.mlabel
+            labels[1] = opts.mlabel
         if opts.tlabel is not None:
-            labels[1] = opts.tlabel
+            labels[0] = opts.tlabel
 
         # Plot extent defaults to calculation extent
         pmin = opts.pmin if opts.pmin is not None else opts.rmin
         pmax = opts.pmax if opts.pmax is not None else opts.rmax
         maglim = opts.maglim
         mag = opts.mag
         l_width = opts.lwidth
-        pdfplot.comparePDFs(
+        plot.compare_funcs(
             pairlist,
             labels,
             rmin=pmin,
@@ -644,9 +644,12 @@ def multiple_targets(parser, opts, pargs, stdout_flag=True):
 
     # Get list of files from target directory
     target_list = list(target_directory.iterdir())
+    to_remove = []
     for target in target_list:
         if target.is_dir():
-            target_list.remove(target)
+            to_remove.append(target)
+    for target in to_remove:
+        target_list.remove(target)
 
     # Do not morph morph_file against itself if it is in the same directory
     if morph_file in target_list:
@@ -782,13 +785,9 @@ def multiple_targets(parser, opts, pargs, stdout_flag=True):
         else:
             try:
                 if field_list is not None:
-                    pdfplot.plot_param(
-                        field_list, param_list, param_name, field
-                    )
+                    plot.plot_param(field_list, param_list, param_name, field)
                 else:
-                    pdfplot.plot_param(
-                        target_file_names, param_list, param_name
-                    )
+                    plot.plot_param(target_file_names, param_list, param_name)
             # Can occur for non-refined plotting parameters
             # i.e. --smear is not selected as an option, but smear is the
             # plotting parameter
@@ -828,9 +827,12 @@ def multiple_morphs(parser, opts, pargs, stdout_flag=True):
 
     # Get list of files from morph directory
     morph_list = list(morph_directory.iterdir())
+    to_remove = []
     for morph in morph_list:
         if morph.is_dir():
-            morph_list.remove(morph)
+            to_remove.append(morph)
+    for morph in to_remove:
+        morph_list.remove(morph)
 
     # Do not morph target_file against itself if it is in the same directory
     if target_file in morph_list:
@@ -967,13 +969,9 @@ def multiple_morphs(parser, opts, pargs, stdout_flag=True):
         else:
             try:
                 if field_list is not None:
-                    pdfplot.plot_param(
-                        field_list, param_list, param_name, field
-                    )
+                    plot.plot_param(field_list, param_list, param_name, field)
                 else:
-                    pdfplot.plot_param(
-                        morph_file_names, param_list, param_name
-                    )
+                    plot.plot_param(morph_file_names, param_list, param_name)
             # Can occur for non-refined plotting parameters
             # i.e. --smear is not selected as an option, but smear is the
             # plotting parameter

src/diffpy/morph/pdfplot.py renamed to src/diffpy/morph/plot.py

@@ -12,7 +12,7 @@
 # See LICENSE.txt for license information.
 #
 ##############################################################################
-"""Collection of plotting functions for PDFs."""
+"""Collection of plotting functions (originally specifically) for PDFs."""
 
 import matplotlib.pyplot as plt
 import numpy
@@ -23,8 +23,8 @@
 
 # FIXME - make this return the figure object in the future, so several views
 # can be composed.
-def plotPDFs(pairlist, labels=None, offset="auto", rmin=None, rmax=None):
-    """Plots several PDFs on top of one another.
+def plot_funcs(pairlist, labels=None, offset="auto", rmin=None, rmax=None):
+    """Plots several functions g(r) on top of one another.
 
     Parameters
     ----------
@@ -34,15 +34,15 @@ def plotPDFs(pairlist, labels=None, offset="auto", rmin=None, rmax=None):
         Iterable of names for the pairs. If this is not the same length as
         the pairlist, a legend will not be shown (default []).
     offset
-        Offset to place between plots. PDFs will be sequentially shifted in
-        the y-direction by the offset. If offset is 'auto' (default), the
+        Offset to place between plots. Functions will be sequentially shifted
+        in the y-direction by the offset. If offset is 'auto' (default), the
         optimal offset will be determined automatically.
     rmin
         The minimum r-value to plot. If this is None (default), the lower
-        bound of the PDF is not altered.
+        bound of the function is not altered.
     rmax
         The maximum r-value to plot. If this is None (default), the upper
-        bound of the PDF is not altered.
+        bound of the function is not altered.
     """
     if labels is None:
         labels = []
@@ -68,7 +68,7 @@ def plotPDFs(pairlist, labels=None, offset="auto", rmin=None, rmax=None):
     return
 
 
-def comparePDFs(
+def compare_funcs(
     pairlist,
     labels=None,
     rmin=None,
@@ -80,10 +80,10 @@ def comparePDFs(
     legend=True,
     l_width=1.5,
 ):
-    """Plot two PDFs on top of each other and difference curve.
+    """Plot two functions g(r) on top of each other and difference curve.
 
-    The second PDF will be shown as blue circles below and the first as a red
-    line. The difference curve will be in green and offset for clarity.
+    The second function will be shown as blue circles below and the first as
+    a red line. The difference curve will be in green and offset for clarity.
 
     Parameters
     ----------
@@ -94,10 +94,10 @@ def comparePDFs(
         the pairlist, a legend will not be shown (default []).
     rmin
         The minimum r-value to plot. If this is None (default), the lower
-        bound of the PDF is not altered.
+        bound of the function is not altered.
     rmax
         The maximum r-value to plot. If this is None (default), the upper
-        bound of the PDF is not altered.
+        bound of the function is not altered.
     show
         Show the plot (default True)
     maglim
@@ -158,7 +158,7 @@ def comparePDFs(
     offset = -1.1 * (ymax - ymin)
 
     # Scale the x-limit based on the r-extent of the signal. This gives a nice
-    # density of PDF peaks.
+    # density of function peaks.
     rlim = rvmax - rvmin
     scale = rlim / 25.0
     # Set a reasonable minimum of .8 and maximum of 1
@@ -305,21 +305,21 @@ def plot_param(target_labels, param_list, param_name=None, field=None):
     return
 
 
-def truncatePDFs(r, gr, rmin=None, rmax=None):
-    """Truncate a PDF to specified bounds.
+def truncate_func(r, gr, rmin=None, rmax=None):
+    """Truncate a function g(r) to specified bounds.
 
     Parameters
     ----------
     r
-        r-values of the PDF.
+        The r-values of the function g(r).
     gr
-        PDF g(r) values.
+        Function g(r) values.
     rmin
         The minimum r-value. If this is None (default), the lower bound of
-        the PDF is not altered.
+        the function is not altered.
     rmax
         The maximum r-value. If this is None (default), the upper bound of
-        the PDF is not altered.
+        the function is not altered.
 
     Returns
     -------
@@ -340,7 +340,7 @@ def truncatePDFs(r, gr, rmin=None, rmax=None):
 
 
 def _find_offset(pairlist):
-    """Find an optimal offset between PDFs."""
+    """Find an optimal offset between functions."""
     maxlist = [max(p[1]) for p in pairlist]
     minlist = [min(p[1]) for p in pairlist]
     difflist = numpy.subtract(maxlist[:-1], minlist[1:])

src/diffpy/morph/tools.py

@@ -93,9 +93,9 @@ def getRw(chain):
     # Make sure we put these on the proper grid
     x_morph, y_morph, x_target, y_target = chain.xyallout
     diff = y_target - y_morph
-    rw = numpy.dot(diff, diff)
-    rw /= numpy.dot(y_target, y_target)
-    rw = rw**0.5
+    rw = numpy.dot(x_morph * diff, diff)
+    rw /= numpy.dot(x_morph * y_morph, y_morph)
+    rw = rw
     return rw
 
 

tests/test_morphapp.py

@@ -18,7 +18,7 @@
 nickel_PDF = testdata_dir.joinpath("nickel_ss0.01.cgr")
 serial_JSON = testdata_dir.joinpath("testsequence_serialfile.json")
 
-testsaving_dir = testdata_dir.joinpath("testsaving")
+testsaving_dir = testsequence_dir.joinpath("testsaving")
 test_saving_succinct = testsaving_dir.joinpath("succinct")
 test_saving_verbose = testsaving_dir.joinpath("verbose")
 tssf = testdata_dir.joinpath("testsequence_serialfile.json")

tests/test_morphio.py

@@ -21,7 +21,7 @@
 testdata_dir = tests_dir.joinpath("testdata")
 testsequence_dir = testdata_dir.joinpath("testsequence")
 
-testsaving_dir = testdata_dir.joinpath("testsaving")
+testsaving_dir = testsequence_dir.joinpath("testsaving")
 test_saving_succinct = testsaving_dir.joinpath("succinct")
 test_saving_verbose = testsaving_dir.joinpath("verbose")
 tssf = testdata_dir.joinpath("testsequence_serialfile.json")

tests/test_tools.py

@@ -60,10 +60,16 @@ def test_nn_value(self, setup):
             pytest.approx(tools.nn_value(-value, name=None), abs(-value))
 
     def test_field_sort(self, setup):
-        sequence_files = [*os.listdir(testsequence_dir)]
+        sequence_files = [file for file in Path(testsequence_dir).iterdir()]
+        to_remove = []
+        for file in sequence_files:
+            if file.is_dir():
+                to_remove.append(file)
+        for d in to_remove:
+            sequence_files.remove(d)
         absolute_sf = []
         for file in sequence_files:
-            absolute_sf.append(os.path.join(testsequence_dir, file))
+            absolute_sf.append(Path(testsequence_dir) / file.name)
 
         # Fisher-Yates randomization
         import random
@@ -87,8 +93,8 @@ def test_field_sort(self, setup):
 
         # Temperature sort should produce same result as alphanumerical if
         # leading character is removed
-        sequence_files.sort(key=lambda entry: entry[2:])
-        assert sequence_files == sorted_sequence
+        sequence_files.sort(key=lambda entry: entry.name[2:])
+        assert [file.name for file in sequence_files] == sorted_sequence
 
         # Check temperatures are correct
         assert fvs == [174, 180, 186, 192, 198, 204, 210]
@@ -103,7 +109,7 @@ def test_field_sort(self, setup):
 
         # Reversed sort should match alphanumerical sort
         sequence_files.sort()
-        assert sequence_files == reversed_sequence
+        assert [file.name for file in sequence_files] == reversed_sequence
 
         # Check we get the same sequence when we load header information from
         # a serial file
@@ -116,7 +122,7 @@ def test_field_sort(self, setup):
         metadata_sequence = []
         for path in metadata_path_sequence:
             metadata_sequence.append(path.name)
-        assert sequence_files == metadata_sequence
+        assert [file.name for file in sequence_files] == metadata_sequence
 
         # Check error thrown when field does not exist
         with pytest.raises(KeyError):

tests/testdata/testsaving/succinct/Morph_Reference_Table.txt renamed to tests/testdata/testsequence/testsaving/succinct/Morph_Reference_Table.txt

@@ -10,9 +10,9 @@
 # vshift = None
 
 # Labels: [Target] [Temperature] [Pearson] [Rw]
-f_180K.gr 180.0 0.999810 0.020141
-e_186K.gr 186.0 0.999424 0.034859
-d_192K.gr 192.0 0.998077 0.062392
-c_198K.gr 198.0 0.994409 0.105918
-b_204K.gr 204.0 0.993160 0.117595
-a_210K.gr 210.0 0.992111 0.127100
+f_180K.gr 180.0 0.999810 0.000674
+e_186K.gr 186.0 0.999424 0.002047
+d_192K.gr 192.0 0.998077 0.008405
+c_198K.gr 198.0 0.994409 0.026768
+b_204K.gr 204.0 0.993160 0.031685
+a_210K.gr 210.0 0.992111 0.035072