From 175f0a6f817b69a9f4a66c591446deaac8232f27 Mon Sep 17 00:00:00 2001 From: Vincent Favre-Nicolin Date: Fri, 31 Jul 2026 12:57:13 +0200 Subject: [PATCH 1/7] fix: guard AddPowderPatternDiffraction setup Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/extensions/powderpattern_ext.cpp | 21 ++++++++++++++++++--- tests/test_powderpattern.py | 13 +++++++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/src/extensions/powderpattern_ext.cpp b/src/extensions/powderpattern_ext.cpp index 28646eb..150406b 100644 --- a/src/extensions/powderpattern_ext.cpp +++ b/src/extensions/powderpattern_ext.cpp @@ -24,6 +24,7 @@ #include #undef B0 +#include #include #include @@ -39,6 +40,12 @@ using namespace ObjCryst; namespace { +class PowderPatternDiffractionShim : public PowderPatternDiffraction +{ + public: + using PowderPatternDiffraction::Prepare; +}; + // This creates a C++ PowderPattern object PowderPattern* _CreatePowderPatternFromCIF(bp::object input) @@ -117,11 +124,19 @@ PowderPatternBackground& addppbackground(PowderPattern& pp) PowderPatternDiffraction& addppdiffraction(PowderPattern& pp, Crystal& crst) { - PowderPatternDiffraction* ppc = new PowderPatternDiffraction(); + std::unique_ptr ppc(new PowderPatternDiffractionShim()); ppc->SetCrystal(crst); + ppc->SetParentPowderPattern(pp); + try + { + ppc->Prepare(); + } + catch(...) + { + throw; + } pp.AddPowderPatternComponent(*ppc); - pp.Prepare(); - return *ppc; + return *ppc.release(); } diff --git a/tests/test_powderpattern.py b/tests/test_powderpattern.py index 7acb666..fb5f1c0 100644 --- a/tests/test_powderpattern.py +++ b/tests/test_powderpattern.py @@ -24,6 +24,7 @@ from pyobjcryst.powderpattern import PowderPattern, SpaceGroupExplorer from pyobjcryst.radiation import RadiationType, WavelengthType from pyobjcryst.reflectionprofile import ReflectionProfileType +from testutils import makeCrystal, makeScatterer # ---------------------------------------------------------------------------- @@ -74,6 +75,18 @@ def test_GetPowderPatternX(self): self.assertTrue(np.array_equal([], self.pp.GetPowderPatternX())) return + def test_AddPowderPatternDiffraction_rollback_on_prepare_error(self): + pp = self.pp + crystal = makeCrystal(*makeScatterer()) + pp.SetWavelength(1.54056) + pp.SetPowderPatternPar(np.deg2rad(0.1), np.deg2rad(0.01), 41) + + with self.assertRaisesRegex(ObjCrystException, "no reflections"): + pp.AddPowderPatternDiffraction(crystal) + + self.assertEqual(0, pp.GetNbPowderPatternComponent()) + return + # def test_GetScaleFactor(self): assert False # def test_ImportPowderPattern2ThetaObs(self): assert False # def test_ImportPowderPattern2ThetaObsSigma(self): assert False From 56b2c38efa6ed45a185cbb63990d9fd43e3010d2 Mon Sep 17 00:00:00 2001 From: Vincent Favre-Nicolin Date: Fri, 31 Jul 2026 12:57:32 +0200 Subject: [PATCH 2/7] docs: add news item for pr 94 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- news/94.rst | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 news/94.rst diff --git a/news/94.rst b/news/94.rst new file mode 100644 index 0000000..3fcaafe --- /dev/null +++ b/news/94.rst @@ -0,0 +1,23 @@ +**Added:** + +* + +**Changed:** + +* + +**Deprecated:** + +* + +**Removed:** + +* + +**Fixed:** + +* Fixed `PowderPattern.AddPowderPatternDiffraction()` so a no-reflections failure does not leave a failed diffraction component attached to the powder pattern. + +**Security:** + +* From be1d8253d3b463dd84c6a6c1f535465e5f1a74b4 Mon Sep 17 00:00:00 2001 From: Vincent Favre-Nicolin Date: Fri, 31 Jul 2026 13:01:12 +0200 Subject: [PATCH 3/7] docs: clarify rollback rationale Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/extensions/powderpattern_ext.cpp | 3 +++ tests/test_powderpattern.py | 2 ++ 2 files changed, 5 insertions(+) diff --git a/src/extensions/powderpattern_ext.cpp b/src/extensions/powderpattern_ext.cpp index 150406b..08557ac 100644 --- a/src/extensions/powderpattern_ext.cpp +++ b/src/extensions/powderpattern_ext.cpp @@ -126,6 +126,9 @@ PowderPatternDiffraction& addppdiffraction(PowderPattern& pp, Crystal& crst) { std::unique_ptr ppc(new PowderPatternDiffractionShim()); ppc->SetCrystal(crst); + // Prepare against the target powder-pattern context before final + // registration so a no-reflections failure cannot leave a broken + // partially attached component behind. ppc->SetParentPowderPattern(pp); try { diff --git a/tests/test_powderpattern.py b/tests/test_powderpattern.py index fb5f1c0..1ae731b 100644 --- a/tests/test_powderpattern.py +++ b/tests/test_powderpattern.py @@ -79,6 +79,8 @@ def test_AddPowderPatternDiffraction_rollback_on_prepare_error(self): pp = self.pp crystal = makeCrystal(*makeScatterer()) pp.SetWavelength(1.54056) + # Keep the 2theta window below the first reflection so setup raises + # and we can verify the failed diffraction component is not retained. pp.SetPowderPatternPar(np.deg2rad(0.1), np.deg2rad(0.01), 41) with self.assertRaisesRegex(ObjCrystException, "no reflections"): From 605d6c538da47c546e5713097046e7efecd7a211 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 31 Jul 2026 11:04:36 +0000 Subject: [PATCH 4/7] [pre-commit.ci] auto fixes from pre-commit hooks --- tests/test_powderpattern.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_powderpattern.py b/tests/test_powderpattern.py index 1ae731b..e80ea89 100644 --- a/tests/test_powderpattern.py +++ b/tests/test_powderpattern.py @@ -18,13 +18,13 @@ import numpy as np import pytest +from testutils import makeCrystal, makeScatterer from pyobjcryst import ObjCrystException from pyobjcryst.indexing import CrystalCentering, CrystalSystem, quick_index from pyobjcryst.powderpattern import PowderPattern, SpaceGroupExplorer from pyobjcryst.radiation import RadiationType, WavelengthType from pyobjcryst.reflectionprofile import ReflectionProfileType -from testutils import makeCrystal, makeScatterer # ---------------------------------------------------------------------------- From 10d2987651bde8811970061affd8b00e91ade754 Mon Sep 17 00:00:00 2001 From: Vincent Favre-Nicolin Date: Fri, 31 Jul 2026 14:28:52 +0200 Subject: [PATCH 5/7] fix: preserve diffraction component typing Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../powderpattern_diffraction_shim.hpp | 21 +++++++++++++++++++ src/extensions/powderpattern_ext.cpp | 8 +------ .../powderpatterndiffraction_ext.cpp | 5 +++++ src/pyobjcryst/powderpattern.py | 7 ++++--- 4 files changed, 31 insertions(+), 10 deletions(-) create mode 100644 src/extensions/powderpattern_diffraction_shim.hpp diff --git a/src/extensions/powderpattern_diffraction_shim.hpp b/src/extensions/powderpattern_diffraction_shim.hpp new file mode 100644 index 0000000..3bff9bd --- /dev/null +++ b/src/extensions/powderpattern_diffraction_shim.hpp @@ -0,0 +1,21 @@ +/***************************************************************************** +* +* pyobjcryst +* +* See AUTHORS.txt for a list of people who contributed. +* See LICENSE.txt for license information. +* +******************************************************************************/ + +#ifndef PYOBJCRYST_POWDERPATTERN_DIFFRACTION_SHIM_HPP +#define PYOBJCRYST_POWDERPATTERN_DIFFRACTION_SHIM_HPP + +#include + +class PowderPatternDiffractionShim : public ObjCryst::PowderPatternDiffraction +{ + public: + using ObjCryst::PowderPatternDiffraction::Prepare; +}; + +#endif diff --git a/src/extensions/powderpattern_ext.cpp b/src/extensions/powderpattern_ext.cpp index 08557ac..2a67dce 100644 --- a/src/extensions/powderpattern_ext.cpp +++ b/src/extensions/powderpattern_ext.cpp @@ -33,6 +33,7 @@ #include "python_streambuf.hpp" #include "helpers.hpp" +#include "powderpattern_diffraction_shim.hpp" namespace bp = boost::python; using namespace boost::python; @@ -40,13 +41,6 @@ using namespace ObjCryst; namespace { -class PowderPatternDiffractionShim : public PowderPatternDiffraction -{ - public: - using PowderPatternDiffraction::Prepare; -}; - - // This creates a C++ PowderPattern object PowderPattern* _CreatePowderPatternFromCIF(bp::object input) { diff --git a/src/extensions/powderpatterndiffraction_ext.cpp b/src/extensions/powderpatterndiffraction_ext.cpp index 50ca04b..58860f1 100644 --- a/src/extensions/powderpatterndiffraction_ext.cpp +++ b/src/extensions/powderpatterndiffraction_ext.cpp @@ -27,6 +27,8 @@ #include #include +#include "powderpattern_diffraction_shim.hpp" + namespace bp = boost::python; using namespace boost::python; using namespace ObjCryst; @@ -77,4 +79,7 @@ void wrap_powderpatterndiffraction() .def("GetFhklObsSq", &PowderPatternDiffraction::GetFhklObsSq, return_value_policy()) ; + + class_ >( + "_PowderPatternDiffractionShim", no_init); } diff --git a/src/pyobjcryst/powderpattern.py b/src/pyobjcryst/powderpattern.py index f930482..873c948 100644 --- a/src/pyobjcryst/powderpattern.py +++ b/src/pyobjcryst/powderpattern.py @@ -390,10 +390,11 @@ def quick_fit_profile( if pdiff is None: # Probably just one diffraction phase, select it for i in range(self.GetNbPowderPatternComponent()): - if isinstance( - self.GetPowderPatternComponent(i), PowderPatternDiffraction + comp = self.GetPowderPatternComponent(i) + if isinstance(comp, PowderPatternDiffraction) or ( + comp.GetClassName() == "PowderPatternDiffraction" ): - pdiff = self.GetPowderPatternComponent(i) + pdiff = comp break if verbose: print( From c9c8854cbba64289f912b11f665d93e7491e0993 Mon Sep 17 00:00:00 2001 From: Vincent Favre-Nicolin Date: Fri, 31 Jul 2026 14:42:02 +0200 Subject: [PATCH 6/7] refactor: simplify addppdiffraction setup Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../powderpattern_diffraction_shim.hpp | 21 ------------------- src/extensions/powderpattern_ext.cpp | 14 ++++++------- .../powderpatterndiffraction_ext.cpp | 4 ---- src/pyobjcryst/powderpattern.py | 7 +++---- 4 files changed, 9 insertions(+), 37 deletions(-) delete mode 100644 src/extensions/powderpattern_diffraction_shim.hpp diff --git a/src/extensions/powderpattern_diffraction_shim.hpp b/src/extensions/powderpattern_diffraction_shim.hpp deleted file mode 100644 index 3bff9bd..0000000 --- a/src/extensions/powderpattern_diffraction_shim.hpp +++ /dev/null @@ -1,21 +0,0 @@ -/***************************************************************************** -* -* pyobjcryst -* -* See AUTHORS.txt for a list of people who contributed. -* See LICENSE.txt for license information. -* -******************************************************************************/ - -#ifndef PYOBJCRYST_POWDERPATTERN_DIFFRACTION_SHIM_HPP -#define PYOBJCRYST_POWDERPATTERN_DIFFRACTION_SHIM_HPP - -#include - -class PowderPatternDiffractionShim : public ObjCryst::PowderPatternDiffraction -{ - public: - using ObjCryst::PowderPatternDiffraction::Prepare; -}; - -#endif diff --git a/src/extensions/powderpattern_ext.cpp b/src/extensions/powderpattern_ext.cpp index 2a67dce..b492fd4 100644 --- a/src/extensions/powderpattern_ext.cpp +++ b/src/extensions/powderpattern_ext.cpp @@ -33,7 +33,6 @@ #include "python_streambuf.hpp" #include "helpers.hpp" -#include "powderpattern_diffraction_shim.hpp" namespace bp = boost::python; using namespace boost::python; @@ -118,19 +117,18 @@ PowderPatternBackground& addppbackground(PowderPattern& pp) PowderPatternDiffraction& addppdiffraction(PowderPattern& pp, Crystal& crst) { - std::unique_ptr ppc(new PowderPatternDiffractionShim()); + std::unique_ptr ppc(new PowderPatternDiffraction()); ppc->SetCrystal(crst); // Prepare against the target powder-pattern context before final // registration so a no-reflections failure cannot leave a broken // partially attached component behind. ppc->SetParentPowderPattern(pp); - try + ppc->GenHKLFullSpace(); + if(ppc->GetNbReflBelowMaxSinThetaOvLambda() == 0) { - ppc->Prepare(); - } - catch(...) - { - throw; + throw ObjCrystException( + "PowderPatternDiffraction::CalcSinThetaLambda(): there are no reflections!" + ); } pp.AddPowderPatternComponent(*ppc); return *ppc.release(); diff --git a/src/extensions/powderpatterndiffraction_ext.cpp b/src/extensions/powderpatterndiffraction_ext.cpp index 58860f1..d29b822 100644 --- a/src/extensions/powderpatterndiffraction_ext.cpp +++ b/src/extensions/powderpatterndiffraction_ext.cpp @@ -27,8 +27,6 @@ #include #include -#include "powderpattern_diffraction_shim.hpp" - namespace bp = boost::python; using namespace boost::python; using namespace ObjCryst; @@ -80,6 +78,4 @@ void wrap_powderpatterndiffraction() return_value_policy()) ; - class_ >( - "_PowderPatternDiffractionShim", no_init); } diff --git a/src/pyobjcryst/powderpattern.py b/src/pyobjcryst/powderpattern.py index 873c948..f930482 100644 --- a/src/pyobjcryst/powderpattern.py +++ b/src/pyobjcryst/powderpattern.py @@ -390,11 +390,10 @@ def quick_fit_profile( if pdiff is None: # Probably just one diffraction phase, select it for i in range(self.GetNbPowderPatternComponent()): - comp = self.GetPowderPatternComponent(i) - if isinstance(comp, PowderPatternDiffraction) or ( - comp.GetClassName() == "PowderPatternDiffraction" + if isinstance( + self.GetPowderPatternComponent(i), PowderPatternDiffraction ): - pdiff = comp + pdiff = self.GetPowderPatternComponent(i) break if verbose: print( From edefe29f14246acae822c5d28b2333a20b2b12c3 Mon Sep 17 00:00:00 2001 From: Vincent Favre-Nicolin Date: Fri, 31 Jul 2026 14:46:21 +0200 Subject: [PATCH 7/7] fix: run prepare after adding diffraction Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/extensions/powderpattern_ext.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/extensions/powderpattern_ext.cpp b/src/extensions/powderpattern_ext.cpp index b492fd4..c40f71a 100644 --- a/src/extensions/powderpattern_ext.cpp +++ b/src/extensions/powderpattern_ext.cpp @@ -131,6 +131,15 @@ PowderPatternDiffraction& addppdiffraction(PowderPattern& pp, Crystal& crst) ); } pp.AddPowderPatternComponent(*ppc); + try + { + pp.Prepare(); + } + catch(...) + { + pp.RemovePowderPatternComponent(*ppc); + throw; + } return *ppc.release(); }