From 0c8d1dc2dcc19c1be8781d9c29223cc6016febde Mon Sep 17 00:00:00 2001 From: BM804 <112770884+BM804@users.noreply.github.com> Date: Fri, 16 Oct 2026 22:36:48 +0530 Subject: [PATCH 1/4] Fix missing magnet model error recursion --- pyaml/magnet/cfm_magnet.py | 4 ++-- pyaml/magnet/magnet.py | 4 ++-- pyaml/magnet/serialized_magnet.py | 4 ++-- tests/magnet/test_missing_model_errors.py | 15 +++++++++++++++ 4 files changed, 21 insertions(+), 6 deletions(-) create mode 100644 tests/magnet/test_missing_model_errors.py diff --git a/pyaml/magnet/cfm_magnet.py b/pyaml/magnet/cfm_magnet.py index 83c5578f..47140100 100644 --- a/pyaml/magnet/cfm_magnet.py +++ b/pyaml/magnet/cfm_magnet.py @@ -239,7 +239,7 @@ def strengths(self) -> abstract.ReadWriteFloatScalar: """ self.check_peer() if self.__strengths is None: - raise PyAMLException(f"{str(self)} has no model that supports physics units") + raise PyAMLException(f"{self.name} has no model that supports physics units") return self.__strengths @property @@ -250,7 +250,7 @@ def hardwares(self) -> abstract.ReadWriteFloatScalar: """ self.check_peer() if self.__hardwares is None: - raise PyAMLException(f"{str(self)} has no model that supports hardware units") + raise PyAMLException(f"{self.name} has no model that supports hardware units") return self.__hardwares def set_energy(self, E: float): diff --git a/pyaml/magnet/magnet.py b/pyaml/magnet/magnet.py index b01491d7..fd5a4a09 100644 --- a/pyaml/magnet/magnet.py +++ b/pyaml/magnet/magnet.py @@ -83,7 +83,7 @@ def strength(self) -> abstract.ReadWriteFloatScalar: """ self.check_peer() if self.__strength is None: - raise PyAMLException(f"{str(self)} has no model that supports physics units") + raise PyAMLException(f"{self.name} has no model that supports physics units") return self.__strength @property @@ -94,7 +94,7 @@ def hardware(self) -> abstract.ReadWriteFloatScalar: """ self.check_peer() if self.__hardware is None: - raise PyAMLException(f"{str(self)} has no model that supports hardware units") + raise PyAMLException(f"{self.name} has no model that supports hardware units") return self.__hardware @property diff --git a/pyaml/magnet/serialized_magnet.py b/pyaml/magnet/serialized_magnet.py index ddfe1733..c5aab6fa 100644 --- a/pyaml/magnet/serialized_magnet.py +++ b/pyaml/magnet/serialized_magnet.py @@ -355,7 +355,7 @@ def strength(self) -> abstract.ReadWriteFloatScalar: """ self.check_peer() if self.__strengths is None: - raise PyAMLException(f"{str(self)} has no model that supports physics units") + raise PyAMLException(f"{self.name} has no model that supports physics units") return self.__strengths @property @@ -365,7 +365,7 @@ def hardware(self) -> abstract.ReadWriteFloatScalar: """ self.check_peer() if self.__hardwares is None: - raise PyAMLException(f"{str(self)} has no model that supports hardware units") + raise PyAMLException(f"{self.name} has no model that supports hardware units") return self.__hardwares def set_energy(self, energy: float): diff --git a/tests/magnet/test_missing_model_errors.py b/tests/magnet/test_missing_model_errors.py new file mode 100644 index 00000000..37c25b57 --- /dev/null +++ b/tests/magnet/test_missing_model_errors.py @@ -0,0 +1,15 @@ +import pytest + +from pyaml import PyAMLException +from pyaml.magnet.hcorrector import HCorrector + + +@pytest.mark.parametrize( + ("accessor", "unit"), + [("strength", "physics"), ("hardware", "hardware")], +) +def test_missing_model_error_names_magnet_without_repr_recursion(accessor, unit): + magnet = HCorrector("CH1").attach(object(), strength=None, hardware=None) + + with pytest.raises(PyAMLException, match=f"CH1 has no model that supports {unit} units"): + getattr(magnet, accessor) From 492730763ea49bc29b06621329e8826b588d6ba1 Mon Sep 17 00:00:00 2001 From: Babu Midddya <112770884+BM804@users.noreply.github.com> Date: Wed, 21 Oct 2026 01:37:56 +0530 Subject: [PATCH 2/4] Clarify missing magnet model regression test --- tests/magnet/test_missing_model_errors.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/tests/magnet/test_missing_model_errors.py b/tests/magnet/test_missing_model_errors.py index 37c25b57..b07402d0 100644 --- a/tests/magnet/test_missing_model_errors.py +++ b/tests/magnet/test_missing_model_errors.py @@ -4,12 +4,11 @@ from pyaml.magnet.hcorrector import HCorrector -@pytest.mark.parametrize( - ("accessor", "unit"), - [("strength", "physics"), ("hardware", "hardware")], -) -def test_missing_model_error_names_magnet_without_repr_recursion(accessor, unit): +def test_missing_model_errors_name_magnet_without_formatting_it(): magnet = HCorrector("CH1").attach(object(), strength=None, hardware=None) - with pytest.raises(PyAMLException, match=f"CH1 has no model that supports {unit} units"): - getattr(magnet, accessor) + with pytest.raises(PyAMLException, match="CH1 has no model that supports physics units"): + magnet.strength + + with pytest.raises(PyAMLException, match="CH1 has no model that supports hardware units"): + magnet.hardware From f65487073e478fa268aea9f1d9c4884a35d68556 Mon Sep 17 00:00:00 2001 From: Babu Midddya <112770884+BM804@users.noreply.github.com> Date: Wed, 21 Oct 2026 01:42:59 +0530 Subject: [PATCH 3/4] Provide a default Python version for CI matrix --- .github/workflows/unit-tests.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index b0bad8d9..9b6a185f 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -21,8 +21,8 @@ jobs: matrix: python-version: >- ${{ (github.event_name == 'push' || github.event_name == 'workflow_dispatch') - && fromJSON(vars.PYTHON_ALL_SUPPORTED_TEST_VERSIONS) - || fromJSON(vars.PYTHON_DEFAULT_TEST_VERSIONS) }} + && fromJSON(vars.PYTHON_ALL_SUPPORTED_TEST_VERSIONS || '["3.11"]') + || fromJSON(vars.PYTHON_DEFAULT_TEST_VERSIONS || '["3.11"]') }} steps: - uses: actions/checkout@v6 From 988b184835db5e8195a849442549e9c14ce86863 Mon Sep 17 00:00:00 2001 From: Alexis Gamelin Date: Mon, 26 Oct 2026 13:27:09 +0100 Subject: [PATCH 4/4] Skip excluded properties in repr and fix BPM error messages Excluded properties are no longer evaluated by __pyaml_repr__, so a getter raising with str(self) cannot re-enter the repr. Apply the same self.name fix to BPM accessors, add regression tests, and revert the unrelated CI change. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/unit-tests.yml | 4 ++-- pyaml/common/element.py | 12 +++++++----- pyaml/diagnostics/bpm/bpm.py | 6 +++--- tests/diagnostics/bpm/test_bpm.py | 12 ++++++++++++ tests/magnet/test_missing_model_errors.py | 6 ++++-- 5 files changed, 28 insertions(+), 12 deletions(-) diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index 9b6a185f..b0bad8d9 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -21,8 +21,8 @@ jobs: matrix: python-version: >- ${{ (github.event_name == 'push' || github.event_name == 'workflow_dispatch') - && fromJSON(vars.PYTHON_ALL_SUPPORTED_TEST_VERSIONS || '["3.11"]') - || fromJSON(vars.PYTHON_DEFAULT_TEST_VERSIONS || '["3.11"]') }} + && fromJSON(vars.PYTHON_ALL_SUPPORTED_TEST_VERSIONS) + || fromJSON(vars.PYTHON_DEFAULT_TEST_VERSIONS) }} steps: - uses: actions/checkout@v6 diff --git a/pyaml/common/element.py b/pyaml/common/element.py index cdf83239..d5b6e6e4 100644 --- a/pyaml/common/element.py +++ b/pyaml/common/element.py @@ -62,11 +62,13 @@ def _class_exclusions(obj) -> set[str]: return exclusions -def _properties(obj) -> dict[str, Any]: +def _properties(obj, exclusions: set[str] = frozenset()) -> dict[str, Any]: + # Excluded properties are skipped before evaluation: a getter that raises + # with str(self) in its message would otherwise re-enter the repr. values: dict[str, Any] = {} for cls in reversed(type(obj).__mro__): for name, descriptor in vars(cls).items(): - if name.startswith("_") or not isinstance(descriptor, property): + if name.startswith("_") or name in exclusions or not isinstance(descriptor, property): continue try: values[name] = getattr(obj, name) @@ -75,7 +77,7 @@ def _properties(obj) -> dict[str, Any]: return values -def _fields(obj) -> dict[str, Any]: +def _fields(obj, exclusions: set[str] = frozenset()) -> dict[str, Any]: custom_fields = getattr(obj, "_pyaml_repr_fields", None) if custom_fields is not None: try: @@ -83,7 +85,7 @@ def _fields(obj) -> dict[str, Any]: except Exception as error: return {"value": _unavailable(error)} - values = _properties(obj) + values = _properties(obj, exclusions) for name, value in vars(obj).items(): if not name.startswith("_"): values.setdefault(name, value) @@ -158,10 +160,10 @@ def _format_object(obj, depth: int, active: set[int], extra_exclusions: set[str] active.add(id(obj)) try: - values = _fields(obj) exclusions = _class_exclusions(obj) if extra_exclusions: exclusions.update(extra_exclusions) + values = _fields(obj, exclusions) parts = [] for name, value in values.items(): if name.startswith("_") or name in exclusions or callable(value): diff --git a/pyaml/diagnostics/bpm/bpm.py b/pyaml/diagnostics/bpm/bpm.py index c63f662a..1f432ad5 100644 --- a/pyaml/diagnostics/bpm/bpm.py +++ b/pyaml/diagnostics/bpm/bpm.py @@ -116,7 +116,7 @@ def positions(self) -> ReadFloatArray: If positions have not been attached """ if self._positions is None: - raise PyAMLException(f"{str(self)} has no attached positions") + raise PyAMLException(f"{self.name} has no attached positions") return self._positions @property @@ -135,7 +135,7 @@ def offset(self) -> ReadWriteFloatArray: If offset has not been attached """ if self._offset is None: - raise PyAMLException(f"{str(self)} has no attached offset") + raise PyAMLException(f"{self.name} has no attached offset") return self._offset @property @@ -154,7 +154,7 @@ def tilt(self) -> ReadWriteFloatScalar: If tilt has not been attached """ if self._tilt is None: - raise PyAMLException(f"{str(self)} has no attached tilt") + raise PyAMLException(f"{self.name} has no attached tilt") return self._tilt def attach( diff --git a/tests/diagnostics/bpm/test_bpm.py b/tests/diagnostics/bpm/test_bpm.py index 3bf8ee71..322ec6fb 100644 --- a/tests/diagnostics/bpm/test_bpm.py +++ b/tests/diagnostics/bpm/test_bpm.py @@ -2,6 +2,8 @@ import pytest from pyaml.accelerator import Accelerator +from pyaml.common.exception import PyAMLException +from pyaml.diagnostics.bpm.bpm import BPM def test_simulator_bpm_tilt(): @@ -67,3 +69,13 @@ def test_simulator_bpm_position_with_bad_corrector_strength(): for bpm in [bpm1, bpm_simple, bpm3]: assert bpm.positions.get()[0] != 0.0 assert bpm.positions.get()[1] != 0.0 + + +def test_unattached_bpm_errors_name_bpm_without_formatting_it(): + bpm = BPM(name="BPM1") + + for accessor in ("positions", "offset", "tilt"): + with pytest.raises(PyAMLException, match=f"BPM1 has no attached {accessor}"): + getattr(bpm, accessor) + + assert "BPM1" in repr(bpm) diff --git a/tests/magnet/test_missing_model_errors.py b/tests/magnet/test_missing_model_errors.py index b07402d0..7ae1d368 100644 --- a/tests/magnet/test_missing_model_errors.py +++ b/tests/magnet/test_missing_model_errors.py @@ -8,7 +8,9 @@ def test_missing_model_errors_name_magnet_without_formatting_it(): magnet = HCorrector("CH1").attach(object(), strength=None, hardware=None) with pytest.raises(PyAMLException, match="CH1 has no model that supports physics units"): - magnet.strength + _ = magnet.strength with pytest.raises(PyAMLException, match="CH1 has no model that supports hardware units"): - magnet.hardware + _ = magnet.hardware + + assert "CH1" in repr(magnet)