From 72845bb716f79eca27a1ebd757e81a7f33667631 Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Wed, 28 Oct 2026 13:35:08 -0400 Subject: [PATCH 1/3] mild test refactor for readability and pre-commit fix --- .pre-commit-config.yaml | 2 -- tests/test_profiles.py | 10 ++++------ 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index a9fa4c7..f10e6c5 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,5 +1,3 @@ -default_language_version: - python: python3.13 ci: autofix_commit_msg: | [pre-commit.ci] auto fixes from pre-commit hooks diff --git a/tests/test_profiles.py b/tests/test_profiles.py index 5d03a66..4c6bffd 100644 --- a/tests/test_profiles.py +++ b/tests/test_profiles.py @@ -227,12 +227,10 @@ def test_update_all_parameters( profile.update(pbso4_pattern.pattern, pbso4_pattern.diffraction) # Read every value back from the installed pyobjcryst profile installed_profile = pbso4_pattern.diffraction.GetProfile() - actual_pyobjcryst_values = { - pyobjcryst_par_name: installed_profile.GetPar( - pyobjcryst_par_name - ).GetHumanValue() - for pyobjcryst_par_name in expected_pyobjcryst_values - } + actual_pyobjcryst_values = {} + for pyobjcryst_par_name in expected_pyobjcryst_values: + parameter = installed_profile.GetPar(pyobjcryst_par_name) + actual_pyobjcryst_values[pyobjcryst_par_name] = parameter.GetHumanValue() assert actual_pyobjcryst_values == pytest.approx( expected_pyobjcryst_values ) From 365f1c6e26d3ce3f2d7b2236dd296eb1263a221c Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Wed, 28 Oct 2026 13:35:39 -0400 Subject: [PATCH 2/3] minor fix --- tests/test_profiles.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test_profiles.py b/tests/test_profiles.py index 4c6bffd..b7fe199 100644 --- a/tests/test_profiles.py +++ b/tests/test_profiles.py @@ -230,7 +230,9 @@ def test_update_all_parameters( actual_pyobjcryst_values = {} for pyobjcryst_par_name in expected_pyobjcryst_values: parameter = installed_profile.GetPar(pyobjcryst_par_name) - actual_pyobjcryst_values[pyobjcryst_par_name] = parameter.GetHumanValue() + actual_pyobjcryst_values[pyobjcryst_par_name] = ( + parameter.GetHumanValue() + ) assert actual_pyobjcryst_values == pytest.approx( expected_pyobjcryst_values ) From bfcd3186ee1e60251bb634cfacdc8c5df1024c52 Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Wed, 28 Oct 2026 14:43:06 -0400 Subject: [PATCH 3/3] feat: add Parameter targets and a settings counter to Component --- news/component-targets-and-settings.rst | 24 +++++ src/diffpy/cmirietveld/component.py | 134 +++++++++++++++++++---- tests/conftest.py | 16 +++ tests/test_component.py | 138 ++++++++++++++++++++++++ 4 files changed, 289 insertions(+), 23 deletions(-) create mode 100644 news/component-targets-and-settings.rst diff --git a/news/component-targets-and-settings.rst b/news/component-targets-and-settings.rst new file mode 100644 index 0000000..4d909b1 --- /dev/null +++ b/news/component-targets-and-settings.rst @@ -0,0 +1,24 @@ +**Added:** + +* Add ``Component.settings_version`` and ``Component.mark_settings_changed``, so a generator can run ``install`` again after a setting of a component changes. +* Add a ``target`` argument to ``Component.create_parameters_from`` and ``Component.push_parameters``, so one component can configure several pyobjcryst objects, such as a pattern and its radiation. + +**Changed:** + +* + +**Deprecated:** + +* + +**Removed:** + +* + +**Fixed:** + +* + +**Security:** + +* diff --git a/src/diffpy/cmirietveld/component.py b/src/diffpy/cmirietveld/component.py index de5cb74..f98425d 100644 --- a/src/diffpy/cmirietveld/component.py +++ b/src/diffpy/cmirietveld/component.py @@ -33,15 +33,86 @@ class Component(ParameterSet): they configure. They do not keep a reference to the pyobjcryst objects they are given, so one component can serve several generators. + A component has two kinds of configuration: + + - **Parameters** are the values a fit can refine, such as a zero shift + or a peak width. They are created from a pyobjcryst model with + ``create_parameters_from``, and ``update`` writes them into the + model with ``push_parameters`` before every calculation. A component + that configures more than one pyobjcryst object, such as a pattern + and its radiation, creates the Parameters of each object under its + own target and pushes them one target at a time. + - **Settings** are the configuration that is not refined, such as the + radiation type or the x-ray tube. ``install`` applies them. A method + that changes a setting calls ``mark_settings_changed``, so every + generator holding the component runs ``install`` again before its + next calculation. + Parameters ---------- name : str The name of the component. It must be a valid attribute name. + + Examples + -------- + An instrument whose ``zero`` belongs to the pattern and whose + ``wavelength`` belongs to the pattern's radiation: + + >>> class MyInstrument(Component): + ... def __init__(self): + ... super().__init__("instrument") + ... self.create_parameters_from( + ... PowderPattern(), {"zero": "Zero"}, target="pattern" + ... ) + ... self.create_parameters_from( + ... Radiation(), + ... {"wavelength": "Wavelength"}, + ... target="radiation", + ... ) + ... + ... def update(self, pattern, diffraction): + ... self.push_parameters(pattern, target="pattern") + ... self.push_parameters( + ... pattern.GetRadiation(), target="radiation" + ... ) """ def __init__(self, name): ParameterSet.__init__(self, name) self._refinable_par_names = {} + self._settings_version = 0 + + @property + def settings_version(self): + """The number of times the settings of the component changed. + + A generator compares it with the value it last installed, and + runs ``install`` again when it differs. It starts at 0, and only + ``mark_settings_changed`` changes it: a change to a Parameter + value is not a settings change. + """ + return self._settings_version + + def mark_settings_changed(self): + """Record that a setting of the component changed. + + Settings are the configuration of a component that is not a + Parameter, such as the radiation type of an instrument. Call + this from every method that changes a setting. It bumps + ``settings_version`` and notifies the component's observers, so + every generator holding the component drops its cached pattern + and runs ``install`` again before its next calculation. + + Examples + -------- + A setting method of an instrument: + + >>> def set_xray_tube(self, element): + ... self._xray_tube = element + ... self.mark_settings_changed() + """ + self._settings_version += 1 + self._flush(other=(self,)) def install(self, pattern, diffraction): """Set the model up in one generator. @@ -73,7 +144,7 @@ def update(self, pattern, diffraction): """ raise NotImplementedError - def create_parameters_from(self, refinable_obj, names): + def create_parameters_from(self, refinable_obj, names, target="default"): """Create the Parameters of the component from a pyobjcryst model. @@ -86,9 +157,9 @@ def create_parameters_from(self, refinable_obj, names): rather than copied, so the names users see are this package's, independent of ObjCryst's. Some ObjCryst names are not valid Parameter names (``2ThetaDispl``). The mapping keeps ObjCryst - parameters out of refinement scripts. The mapping is stored, so - ``push_parameters`` writes each value back to the same - ``RefinablePar``. + parameters out of refinement scripts. The mapping is stored under + ``target``, so ``push_parameters`` writes each value back to the + same ``RefinablePar``. Parameters ---------- @@ -96,8 +167,13 @@ def create_parameters_from(self, refinable_obj, names): A fresh instance of the pyobjcryst model. names : dict of str to str The ``RefinablePar`` name for each Parameter name, for example - ``{"eta0": "Eta0"}``. Only the named ``RefinablePar``s get a - Parameter. + ``{"eta0": "Eta0"}``. Only the named ``RefinablePar`` objects + get a Parameter. + target : str, optional + The name of the pyobjcryst object the Parameters belong to + (default "default"). A component that configures more than one + pyobjcryst object, such as a pattern and its radiation, creates + the Parameters of each object under its own target. Raises ------ @@ -134,13 +210,14 @@ def create_parameters_from(self, refinable_obj, names): refinable_par.GetHumanMin(), refinable_par.GetHumanMax() ) self.add_parameter(parameter) - self._refinable_par_names.update(names) + self._refinable_par_names.setdefault(target, {}).update(names) - def push_parameters(self, refinable_obj): + def push_parameters(self, refinable_obj, target="default"): """Push the Parameter values into a pyobjcryst model. - Each value is written to the ``RefinablePar`` that - ``create_parameters_from`` mapped it to. + Each value of the target's Parameters is written to the + ``RefinablePar`` that ``create_parameters_from`` mapped it to. The + Parameters of other targets are neither checked nor pushed. A value outside the limits of its ``RefinablePar`` raises an error instead of being clamped, because ObjCryst would otherwise use a @@ -150,30 +227,41 @@ def push_parameters(self, refinable_obj): value past a limit. An optimizer run without the recipe's bounds stops with this error the first time it steps past a limit. - All the values are checked before any is pushed, so an error - leaves the model unchanged rather than half updated. + All the values of the target are checked before any is pushed, + so an error leaves the model unchanged rather than half updated. + A component with several targets pushes them one at a time, so an + error in a later target leaves the earlier targets' objects + updated. The calculation stops with the error, and the next one + pushes every value again. Parameters ---------- refinable_obj : pyobjcryst.refinableobj.RefinableObj The pyobjcryst model to update. + target : str, optional + The target whose Parameters are pushed (default "default"), as + given to ``create_parameters_from``. Only that target's values + are checked and pushed. Raises ------ ValueError - If a Parameter value is outside the limits of its - ``RefinablePar``. + If the component has no Parameters for the target, or a + Parameter value is outside the limits of its ``RefinablePar``. """ - values = { - parameter_name: self.get(parameter_name).value - for parameter_name in self._refinable_par_names - } - refinable_pars = { - parameter_name: refinable_obj.GetPar(refinable_par_name) - for parameter_name, refinable_par_name in ( - self._refinable_par_names.items() + if target not in self._refinable_par_names: + raise ValueError( + f"The component '{self.name}' has no Parameters for the " + f"target '{target}'. Please use one of its targets: " + f"{', '.join(self._refinable_par_names)}." ) - } + refinable_par_names = self._refinable_par_names[target] + values = {} + refinable_pars = {} + for parameter_name, refinable_par_name in refinable_par_names.items(): + values[parameter_name] = self.get(parameter_name).value + refinable_par = refinable_obj.GetPar(refinable_par_name) + refinable_pars[parameter_name] = refinable_par for parameter_name, refinable_par in refinable_pars.items(): if not refinable_par.IsLimited(): continue diff --git a/tests/conftest.py b/tests/conftest.py index 8f5667c..505004a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -43,6 +43,22 @@ def calculate(self): return calculated_pattern +class NotificationCounter: + """An observer that counts the notifications it receives.""" + + def __init__(self): + self.count = 0 + + def __call__(self, semaphores): + self.count += 1 + + +@pytest.fixture +def notification_counter(): + notification_counter = NotificationCounter() + return notification_counter + + @pytest.fixture def pbso4_pattern(): pbso4_pattern = PbSO4Pattern() diff --git a/tests/test_component.py b/tests/test_component.py index cf155ad..7e2b1a9 100644 --- a/tests/test_component.py +++ b/tests/test_component.py @@ -1,6 +1,8 @@ import re import pytest +from pyobjcryst.powderpattern import PowderPattern +from pyobjcryst.radiation import Radiation from pyobjcryst.reflectionprofile import ReflectionProfilePseudoVoigt from diffpy.cmirietveld.component import Component @@ -190,3 +192,139 @@ def test_push_parameters_outside_narrowed_bounds(pseudo_voigt_component): actual_value = installed_profile.GetPar("W").GetHumanValue() expected_value = 0.5 assert actual_value == pytest.approx(expected_value) + + +@pytest.mark.parametrize( + "pushed_targets, expected_zero, expected_wavelength", + [ + # Test that push_parameters writes only the Parameters of the + # target it is given, into the object it is given. The component + # has two Parameters from two pyobjcryst objects: zero, from a + # PowderPattern's Zero, under the target "pattern", and wavelength, + # from a Radiation's Wavelength, under the target "radiation". Both + # values are changed, the listed targets are pushed ("pattern" into + # a new pattern, "radiation" into that pattern's radiation), and + # both values are read back from the pattern. A target that isn't + # pushed keeps ObjCryst's defaults, Zero = 0.0 and Wavelength = 1.0. + # C1: Only the pattern target is pushed into the pattern. + # Expected: The pattern's Zero has the new value, and the + # radiation's Wavelength keeps its default, 1.0. + (["pattern"], 0.1, 1.0), + # C2: Only the radiation target is pushed into the radiation. + # Expected: The radiation's Wavelength has the new value, and the + # pattern's Zero keeps its default, 0.0. + (["radiation"], 0.0, 1.5406), + # C3: Both targets are pushed, each into its own object, as an + # instrument's update will do. + # Expected: Both have the new values. + (["pattern", "radiation"], 0.1, 1.5406), + ], +) +def test_push_parameters_with_targets( + pushed_targets, expected_zero, expected_wavelength +): + # Create one Parameter for each target, from fresh pyobjcryst objects + component = Component("instrument") + component.create_parameters_from( + PowderPattern(), {"zero": "Zero"}, target="pattern" + ) + component.create_parameters_from( + Radiation(), {"wavelength": "Wavelength"}, target="radiation" + ) + # Change both values + component.get("zero").set_value(0.1) + component.get("wavelength").set_value(1.5406) + # Push each listed target into its own object of a new pattern + pattern = PowderPattern() + refinable_objs = {"pattern": pattern, "radiation": pattern.GetRadiation()} + for target in pushed_targets: + component.push_parameters(refinable_objs[target], target=target) + # Read both values back from the pattern + actual_zero = pattern.GetPar("Zero").GetHumanValue() + actual_wavelength = ( + pattern.GetRadiation().GetPar("Wavelength").GetHumanValue() + ) + assert actual_zero == pytest.approx(expected_zero) + assert actual_wavelength == pytest.approx(expected_wavelength) + + +def test_push_parameters_unknown_target(): + # Test pushing a target the component has no Parameters for + # Expected: A ValueError names the target and lists the component's + # targets, instead of pushing nothing. Targets are case-sensitive. + + # Create an "instrument" component with one Parameter for + # each of two targets, "pattern" and "radiation". + component = Component("instrument") + component.create_parameters_from( + PowderPattern(), {"zero": "Zero"}, target="pattern" + ) + component.create_parameters_from( + Radiation(), {"wavelength": "Wavelength"}, target="radiation" + ) + expected_msg = ( + "The component 'instrument' has no Parameters for the target " + "'not_a_target'. Please use one of its targets: pattern, radiation." + ) + with pytest.raises(ValueError, match=re.escape(expected_msg)): + component.push_parameters(Radiation(), target="not_a_target") + + +def test_push_parameters_other_target_outside_limits(): + # Test pushing one target while a Parameter of another target is + # outside its limits + # Expected: Only the pushed target's values are checked, so the + # pattern's Zero gets the new value, even though wavelength, 100.0, is + # outside the Wavelength limits [0.05, 20]. + # Create one Parameter for each target, from fresh pyobjcryst objects + component = Component("instrument") + component.create_parameters_from( + PowderPattern(), {"zero": "Zero"}, target="pattern" + ) + component.create_parameters_from( + Radiation(), {"wavelength": "Wavelength"}, target="radiation" + ) + # Change zero, and set wavelength outside its limits + component.get("zero").set_value(0.1) + component.get("wavelength").set_value(100.0) + # Push only the "pattern" target into a new pattern + pattern = PowderPattern() + component.push_parameters(pattern, target="pattern") + actual_zero = pattern.GetPar("Zero").GetHumanValue() + expected_zero = 0.1 + assert actual_zero == pytest.approx(expected_zero) + + +def test_mark_settings_changed(notification_counter): + # Test that each settings change bumps the settings counter and + # notifies the component's observers, as a generator holding it is + # Expected: The counter starts at 0 and counts each change, and the + # observer is notified once per change. + component = Component("instrument") + component.add_observer(notification_counter) + # Read the counter before any change + actual_counts = [(component.settings_version, notification_counter.count)] + # Change the settings twice + component.mark_settings_changed() + actual_counts.append( + (component.settings_version, notification_counter.count) + ) + component.mark_settings_changed() + actual_counts.append( + (component.settings_version, notification_counter.count) + ) + expected_counts = [(0, 0), (1, 1), (2, 2)] + assert actual_counts == expected_counts + + +def test_settings_version_after_parameter_change(): + # Test that changing a Parameter value is not a settings change + # Expected: settings_version stays 0, because Parameter values reach + # ObjCryst through update before every calculation, so no generator + # needs to run install again. + component = Component("instrument") + component.create_parameters_from(PowderPattern(), {"zero": "Zero"}) + component.get("zero").set_value(0.1) + actual_settings_version = component.settings_version + expected_settings_version = 0 + assert actual_settings_version == expected_settings_version