Merge pull request #183 from stevenhua0320/iterpar-behavior

fix: fix iterpars behavior

Author: sbillinge

news/iterpar-behavior.rst

@@ -0,0 +1,23 @@
+**Added:**
+
+* <news item>
+
+**Changed:**
+
+* Changed `iterPars` method to match all equal-type atoms to have same ADPs
+
+**Deprecated:**
+
+* <news item>
+
+**Removed:**
+
+* <news item>
+
+**Fixed:**
+
+* <news item>
+
+**Security:**
+
+* <news item>

src/diffpy/srfit/fitbase/recipeorganizer.py

@@ -240,7 +240,9 @@ def _iter_managed(self):
         """Get iterator over managed objects."""
         return chain(*(d.values() for d in self.__managed))
 
-    def iterate_over_parameters(self, pattern="", recurse=True):
+    def iterate_over_parameters(
+        self, pattern="", recurse=True, fullnames=False
+    ):
         """Iterate over the Parameters contained in this object.
 
         Parameters
@@ -256,6 +258,10 @@ def iterate_over_parameters(self, pattern="", recurse=True):
             (default), the method will also iterate over
             parameters in managed sub-objects. If False, only
             top-level parameters will be iterated over.
+        fullnames : bool, optional
+            The flag indicating whether to match against hierarchical
+            dotted namesrelative to this object.
+            If False (default), match only leaf parameter names.
 
         Example
         -------
@@ -266,22 +272,67 @@ def iterate_over_parameters(self, pattern="", recurse=True):
                 print(f"{param.name}={param.value}")
         """
         regexp = re.compile(pattern)
+        if not fullnames:
+            yield from self._iterpars_leafnames(regexp, pattern, recurse)
+        else:
+            yield from self._iterpars_fullnames(
+                regexp, recurse=recurse, prefix=""
+            )
+
+    def _iter_local_parameters(self, regexp, prefix=""):
+        """Iterate over local Parameters with matching names."""
         for parameter in list(self._parameters.values()):
-            if regexp.search(parameter.name):
+            name = f"{prefix}{parameter.name}"
+            if regexp.search(name):
                 yield parameter
-        if not recurse:
-            return
-        # Iterate over objects within the managed dictionaries.
+
+    def _iter_managed_parameter_containers(self):
+        """Iterate over managed objects that can iterate over
+        Parameters."""
         managed = self.__managed[:]
         managed.remove(self._parameters)
-        for m in managed:
-            for obj in m.values():
+        for managed_dict in managed:
+            for obj in managed_dict.values():
                 if hasattr(obj, "iterate_over_parameters"):
-                    for parameter in obj.iterate_over_parameters(
-                        pattern=pattern
-                    ):
-                        yield parameter
-        return
+                    yield obj
+
+    def _iterpars_leafnames(self, regexp, pattern, recurse=True):
+        """Iterate over Parameters matched by leaf parameter names."""
+        yield from self._iter_local_parameters(regexp)
+        if recurse:
+            for obj in self._iter_managed_parameter_containers():
+                yield from obj.iterate_over_parameters(
+                    pattern=pattern,
+                    recurse=True,
+                )
+
+    def _iter_child_fullname_parameters(self, obj, regexp, prefix):
+        """Iterate over one child's Parameters with hierarchical
+        names."""
+        if hasattr(obj, "_iterpars_fullnames"):
+            childprefix = f"{prefix}{obj.name}."
+            yield from obj._iterpars_fullnames(
+                regexp,
+                recurse=True,
+                prefix=childprefix,
+            )
+        else:
+            yield from obj.iterate_over_parameters(
+                pattern=regexp.pattern,
+                recurse=True,
+            )
+
+    def _iterpars_fullnames(self, regexp, recurse=True, prefix=""):
+        """Iterate over Parameters matched by hierarchical dotted
+        names."""
+        yield from self._iter_local_parameters(regexp, prefix=prefix)
+        if recurse:
+            for obj in self._iter_managed_parameter_containers():
+                yield from self._iter_child_fullname_parameters(
+                    obj,
+                    regexp,
+                    prefix,
+                )
 
     @deprecated(iterPars_deprecation_msg)
     def iterPars(self, pattern="", recurse=True):

tests/test_pdf.py

@@ -24,6 +24,8 @@
 
 from diffpy.srfit.exceptions import SrFitError
 from diffpy.srfit.fitbase import ProfileParser
+from diffpy.srfit.fitbase.parameter import Parameter
+from diffpy.srfit.fitbase.recipeorganizer import RecipeContainer
 from diffpy.srfit.pdf import PDFContribution, PDFGenerator, PDFParser
 
 # ----------------------------------------------------------------------------
@@ -321,3 +323,114 @@ def test_pickling(
 
 if __name__ == "__main__":
     unittest.main()
+
+
+def _make_iterpars_tree():
+    """Build a small hierarchy for iterPars tests."""
+    root = RecipeContainer("root")
+    root._containers = {}
+    root._manage(root._containers)
+
+    root_biso = Parameter("Biso", 10)
+    root._add_object(root_biso, root._parameters)
+
+    ni0 = RecipeContainer("Ni0")
+    ni0_biso = Parameter("Biso", 20)
+    ni0_uiso = Parameter("Uiso", 30)
+    ni0._add_object(ni0_biso, ni0._parameters)
+    ni0._add_object(ni0_uiso, ni0._parameters)
+
+    ni1 = RecipeContainer("Ni1")
+    ni1_biso = Parameter("Biso", 40)
+    ni1._add_object(ni1_biso, ni1._parameters)
+
+    o0 = RecipeContainer("O0")
+    o0_biso = Parameter("Biso", 50)
+    o0._add_object(o0_biso, o0._parameters)
+
+    root._add_object(ni0, root._containers)
+    root._add_object(ni1, root._containers)
+    root._add_object(o0, root._containers)
+
+    return {
+        "root": root,
+        "root_biso": root_biso,
+        "ni0": ni0,
+        "ni0_biso": ni0_biso,
+        "ni0_uiso": ni0_uiso,
+        "ni1": ni1,
+        "ni1_biso": ni1_biso,
+        "o0": o0,
+        "o0_biso": o0_biso,
+    }
+
+
+@pytest.mark.parametrize(
+    ("pattern", "kwargs", "expected_values"),
+    [
+        # C1: Match leaf parameter names without fullnames.
+        # Expected: all Biso parameters in the hierarchy are returned.
+        (r"^Biso$", {}, [10, 20, 40, 50]),
+        # C2: Match hierarchical names without fullnames.
+        # Expected: no leaf names match the hierarchical pattern.
+        (r"^Ni\d+\.Biso$", {}, []),
+        # C3: Match hierarchical names with fullnames enabled.
+        # Expected: matching Ni Biso parameters are returned.
+        (r"^Ni\d+\.Biso$", {"fullnames": True}, [20, 40]),
+        # C4: Match one hierarchical Uiso name.
+        # Expected: only Ni0.Uiso is returned.
+        (r"^Ni0\.Uiso$", {"fullnames": True}, [30]),
+        # C5: Match one hierarchical Biso name outside Ni containers.
+        # Expected: only O0.Biso is returned.
+        (r"^O0\.Biso$", {"fullnames": True}, [50]),
+        # C6: Disable recursion while matching child fullnames.
+        # Expected: no child parameters are returned.
+        (r"^Ni\d+\.Biso$", {"fullnames": True, "recurse": False}, []),
+        # C7: Disable recursion while matching root fullname.
+        # Expected: only the root-level Biso parameter is returned.
+        (r"^Biso$", {"fullnames": True, "recurse": False}, [10]),
+    ],
+)
+def test_iterpars_fullname_matching(pattern, kwargs, expected_values):
+    """Verify leaf-name and fullname matching in
+    iterate_over_parameters."""
+    objs = _make_iterpars_tree()
+    root = objs["root"]
+
+    actual_values = [
+        parameter.value
+        for parameter in root.iterate_over_parameters(pattern, **kwargs)
+    ]
+
+    assert actual_values == expected_values
+
+
+@pytest.mark.parametrize(
+    ("pattern", "expected_name"),
+    [
+        # C1: Match Biso relative to the called Ni0 container.
+        # Expected: Ni0.Biso is returned without the Ni0 prefix.
+        (r"^Biso$", ["Biso"]),
+        # C2: Match Uiso relative to the called Ni0 container.
+        # Expected: Ni0.Uiso is returned without the Ni0 prefix.
+        (r"^Uiso$", ["Uiso"]),
+        # C3: Match with the parent container prefix from inside Ni0.
+        # Expected: no parameter is returned because fullnames are relative
+        # to the container on which iterate_over_parameters is called.
+        (r"^Ni0\.Biso$", []),
+    ],
+)
+def test_iterpars_fullnames_are_relative_to_called_container(
+    pattern,
+    expected_name,
+):
+    """Verify fullname matching is relative to the called container."""
+    objs = _make_iterpars_tree()
+    ni0 = objs["ni0"]
+
+    actual_name = [
+        parameter.name
+        for parameter in ni0.iterate_over_parameters(pattern, fullnames=True)
+    ]
+
+    assert actual_name == expected_name