diff --git a/news/remove-custompqconfig.rst b/news/remove-custompqconfig.rst new file mode 100644 index 0000000..ce6b81d --- /dev/null +++ b/news/remove-custompqconfig.rst @@ -0,0 +1,18 @@ +**Changed:** + +* Structure conversion now returns ordinary atomic or periodic adapters, + without reading ``pdffit`` metadata. Configure PDF parameters explicitly + through the existing calculator interfaces. +* ``nometa()`` returns or converts the source adapter without a proxy. + ``None`` returns the empty adapter. + +**Deprecated:** + +* ``NoMetaStructureAdapter``, retained for saved objects and subclasses. + Its Python constructor now emits ``DeprecationWarning``. Historical + metadata adapter classes remain importable for pickle compatibility. + +**Removed:** + +* The ``_customPQConfig`` bindings and Python callback dispatch, including + automatic application of structure metadata to PDF calculators. diff --git a/src/diffpy/srreal/pdfcalculator.py b/src/diffpy/srreal/pdfcalculator.py index cf776e2..3619378 100644 --- a/src/diffpy/srreal/pdfcalculator.py +++ b/src/diffpy/srreal/pdfcalculator.py @@ -163,8 +163,8 @@ def _defineCommonInterface(cls): def _call_kwargs(self, structure=None, **kwargs): """Calculate PDF for the given structure as an (r, G) tuple. Keyword arguments can be used to configure calculator - attributes, these override any properties that may be passed - from the structure, such as spdiameter. + attributes. Structure pdffit metadata is ignored; pass any + desired settings explicitly. Attributes ---------- @@ -180,9 +180,6 @@ def _call_kwargs(self, structure=None, **kwargs): """ setattrFromKeywordArguments(self, **kwargs) self.eval(structure) - # apply kwargs again if structure contained any attribute - # that may affect the result. - setattrFromKeywordArguments(self, **kwargs) rv = (self.rgrid, self.pdf) return rv diff --git a/src/diffpy/srreal/structureadapter.py b/src/diffpy/srreal/structureadapter.py index 7880795..007142e 100644 --- a/src/diffpy/srreal/structureadapter.py +++ b/src/diffpy/srreal/structureadapter.py @@ -18,9 +18,8 @@ Routines: createStructureAdapter -- create StructureAdapter from a Python object -nometa -- create StructureAdapter with disabled _customPQConfig method - this prevents copying of diffpy.structure pdffit metadata - to PDFCalculator object +nometa -- return or convert a StructureAdapter without a proxy; + structure metadata no longer configures calculators nosymmetry -- create StructureAdapter with disabled symmetry expansion. Constants: diff --git a/src/diffpy/srreal/structureconverters.py b/src/diffpy/srreal/structureconverters.py index 834003f..2a2586f 100644 --- a/src/diffpy/srreal/structureconverters.py +++ b/src/diffpy/srreal/structureconverters.py @@ -43,18 +43,9 @@ def convertDiffPyStructure(stru): """Adapt Structure class from diffpy.structure package.""" haslattice = (1, 1, 1, 90, 90, 90) != stru.lattice.abcABG() isperiodic = haslattice - hasmeta = _DiffPyStructureMetadata.hasMetadata(stru) - if hasmeta: - if isperiodic: - adpt = DiffPyStructurePeriodicAdapter() - else: - adpt = DiffPyStructureAtomicAdapter() - adpt._fetchMetadata(stru) - else: - if isperiodic: - adpt = PeriodicStructureAdapter() - else: - adpt = AtomicStructureAdapter() + adpt = ( + PeriodicStructureAdapter() if isperiodic else AtomicStructureAdapter() + ) _fetchDiffPyStructureData(adpt, stru) return adpt @@ -62,88 +53,14 @@ def convertDiffPyStructure(stru): # Adapter classes and helpers for diffpy.structure class --------------------- -class _DiffPyStructureMetadata(object): - """Base class for handling metadata information in the pdffit - attribute.""" - - pdffit = None - - @staticmethod - def hasMetadata(stru): - """True if Structure object carries data in its pdffit - attribute.""" - rv = hasattr(stru, "pdffit") and bool(stru.pdffit) - return rv - - def _customPQConfig(self, pqobj): - """Apply PDF-related metadata if defined in PDFFit structure - format.""" - pqname = type(pqobj).__name__ - if pqname not in ("PDFCalculator", "DebyePDFCalculator"): - return - if not self.pdffit: - return - # scale - envtps = pqobj.usedenvelopetypes - if "scale" not in envtps: - pqobj.addEnvelope("scale") - pqobj.scale = self.pdffit["scale"] - # spdiameter - if "spdiameter" in self.pdffit: - if "sphericalshape" not in envtps: - pqobj.addEnvelope("sphericalshape") - pqobj.spdiameter = self.pdffit["spdiameter"] - # stepcut - if "stepcut" in self.pdffit: - if "stepcut" not in envtps: - pqobj.addEnvelope("stepcut") - pqobj.stepcut = self.pdffit["stepcut"] - # delta1, delta2 - set these only when using JeongPeakWidth model - if pqobj.peakwidthmodel.type() == "jeong": - pqobj.delta1 = self.pdffit["delta1"] - pqobj.delta2 = self.pdffit["delta2"] - return - - def _fetchMetadata(self, stru): - """Copy data from the pdffit attribute of diffpy Structure - object. - - Attributes - ---------- - stru - instance of Structure class from diffpy.structure - - No return value. - """ - # get PDF-related metadata - self.pdffit = {} - if self.hasMetadata(stru): - self.pdffit.update(scale=1.0, delta1=0.0, delta2=0.0) - self.pdffit.update(stru.pdffit) - return - - -# end of class _DiffPyStructureMetadata - - -def _installDiffPyStructureMetadata(cls): - """Install diffpy.structure metadata hooks without multiple - inheritance.""" - cls.pdffit = _DiffPyStructureMetadata.pdffit - cls.hasMetadata = staticmethod(_DiffPyStructureMetadata.hasMetadata) - cls._customPQConfig = _DiffPyStructureMetadata._customPQConfig - cls._fetchMetadata = _DiffPyStructureMetadata._fetchMetadata - return cls - - -@_installDiffPyStructureMetadata +# Keep the historical class paths so saved adapters can still be restored. +# Their pdffit state, if present in a pickle, is inert. class DiffPyStructureAtomicAdapter(AtomicStructureAdapter): - pass + """Legacy adapter retained for pickle compatibility.""" -@_installDiffPyStructureMetadata class DiffPyStructurePeriodicAdapter(PeriodicStructureAdapter): - pass + """Legacy adapter retained for pickle compatibility.""" def _fetchDiffPyStructureData(adpt, stru): diff --git a/src/extensions/wrap_AtomicStructureAdapter.cpp b/src/extensions/wrap_AtomicStructureAdapter.cpp index cf42e1d..0d7f1ea 100644 --- a/src/extensions/wrap_AtomicStructureAdapter.cpp +++ b/src/extensions/wrap_AtomicStructureAdapter.cpp @@ -346,7 +346,7 @@ class AtomPickleSuite : public SerializationPickleSuite } }; -// template wrapper class for overloading of clone and _customPQConfig +// template wrapper class for overloading of clone and diff template class MakeWrapper : @@ -368,17 +368,6 @@ class MakeWrapper : } - void customPQConfig(PairQuantity* pq) const override - { - NB_OVERRIDE_NAME("_customPQConfig", customPQConfig, pq); - } - - void default_customPQConfig(PairQuantity* pq) const - { - this->T::customPQConfig(pq); - } - - StructureDifference diff(StructureAdapterConstPtr other) const override { nb::gil_scoped_acquire gil; @@ -428,14 +417,6 @@ StructureAdapterPtr atomadapter_clone(const T& self) return wrapper ? wrapper->default_clone() : self.clone(); } -template -void atomadapter_customPQConfig(const T& self, PairQuantity* pq) -{ - auto* wrapper = dynamic_cast*>(&self); - if (wrapper) wrapper->default_customPQConfig(pq); - else self.customPQConfig(pq); -} - // Wrapper helpers for class AtomicStructureAdapter typedef MakeWrapper AtomicStructureAdapterWrap; @@ -730,7 +711,6 @@ void prepare_structure_load(diffpy::srreal::StructureAdapter& adapter) // declare shared docstrings from wrap_StructureAdapter.cpp extern const char* doc_StructureAdapter___init__fromstring; -extern const char* doc_StructureAdapter__customPQConfig; extern const char* doc_StructureAdapter_diff; // Wrapper definitions ------------------------------------------------------- @@ -823,10 +803,6 @@ void wrap_AtomicStructureAdapter(nb::module_& m) .def("clone", atomadapter_clone, doc_AtomicStructureAdapter_clone) - .def("_customPQConfig", - atomadapter_customPQConfig, - nb::arg("pqobj"), - doc_StructureAdapter__customPQConfig) .def("diff", atomadapter_diff, nb::arg("other").none(), @@ -873,10 +849,6 @@ void wrap_AtomicStructureAdapter(nb::module_& m) .def("clone", atomadapter_clone, doc_PeriodicStructureAdapter_clone) - .def("_customPQConfig", - atomadapter_customPQConfig, - nb::arg("pqobj"), - doc_StructureAdapter__customPQConfig) .def("diff", atomadapter_diff, nb::arg("other").none(), @@ -916,10 +888,6 @@ void wrap_AtomicStructureAdapter(nb::module_& m) .def("clone", atomadapter_clone, doc_CrystalStructureAdapter_clone) - .def("_customPQConfig", - atomadapter_customPQConfig, - nb::arg("pqobj"), - doc_StructureAdapter__customPQConfig) .def("diff", atomadapter_diff, nb::arg("other").none(), diff --git a/src/extensions/wrap_StructureAdapter.cpp b/src/extensions/wrap_StructureAdapter.cpp index 9b97cd3..27dcec2 100644 --- a/src/extensions/wrap_StructureAdapter.cpp +++ b/src/extensions/wrap_StructureAdapter.cpp @@ -19,6 +19,7 @@ *****************************************************************************/ #include +#include #include "srreal_trampoline.hpp" #include @@ -129,18 +130,6 @@ i -- zero-based atom site index.\n\ Return a 3 by 3 array.\n\ "; -const char* doc_StructureAdapter__customPQConfig = "\ -Support for optional custom configuration of the PairQuantity object.\n\ -This method is called from the setStructure and eval methods of the owner\n\ -PairQuantity object.\n\ -\n\ -pqobj -- the owner PairQuantity object. The function should check for the\n\ - exact type of pqobj and apply configuration accordingly.\n\ -\n\ -No return value. This method can be overloaded in a derived class.\n\ -No action by default.\n\ -"; - const char* doc_StructureAdapter_diff = "\ Evaluate difference between this and other StructureAdapter.\n\ \n\ @@ -153,14 +142,14 @@ This method can be overloaded in a derived class.\n\ "; const char* doc_NoMetaStructureAdapter = "\ -StructureAdapter proxy which disables _customPQConfig method.\n\ +Deprecated structure proxy retained for compatibility with saved objects.\n\ +Structure metadata no longer configures calculators.\n\ "; const char* doc_NoMetaStructureAdapter_init = "\ -Create proxy to StructureAdapter that disables _customPQConfig.\n\ +Create a deprecated proxy. Use the source adapter directly instead.\n\ \n\ -adapter -- StructureAdapter object to be proxied. The new adapter\n\ - will avoid calling of adapter._customPQConfig method.\n\ +adapter -- source StructureAdapter, or None for an empty structure.\n\ "; const char* doc_NoSymmetryStructureAdapter = "\ @@ -176,13 +165,10 @@ adapter -- StructureAdapter object to be proxied. Any iteration over\n\ "; const char* doc_nometa = "\ -Return a proxy to StructureAdapter with _customPQConfig method disabled.\n\ -This creates a thin wrapper over a source StructureAdapter object that\n\ -disables _customPQConfig.\n\ +Return the structure adapter without creating a metadata proxy.\n\ \n\ -stru -- StructureAdapter object or an object convertible to StructureAdapter.\n\ -\n\ -Return a proxy StructureAdapter with disabled _customPQConfig.\n\ +Existing adapters are returned unchanged. Other supported structures are\n\ +converted normally; None returns EMPTY. Structure metadata is not applied.\n\ "; const char* doc_nosymmetry = "\ @@ -482,19 +468,6 @@ class StructureAdapterWrap : } - void customPQConfig(PairQuantity* pq) const override - { - if (mserialized) return mserialized->customPQConfig(pq); - NB_OVERRIDE_NAME("_customPQConfig", customPQConfig, pq); - } - - void default_customPQConfig(PairQuantity* pq) const - { - if (mserialized) return mserialized->customPQConfig(pq); - this->StructureAdapter::customPQConfig(pq); - } - - StructureDifference diff(StructureAdapterConstPtr other) const override { nb::gil_scoped_acquire gil; @@ -573,13 +546,6 @@ BaseBondGeneratorPtr createBondGenerator_shared(StructureAdapterPtr adpt) return adpt->createBondGenerator(); } -void customPQConfig_dispatch(const StructureAdapter& adpt, PairQuantity* pq) -{ - auto* wrap = dynamic_cast(&adpt); - if (wrap) wrap->default_customPQConfig(pq); - else adpt.customPQConfig(pq); -} - template class StructureProxyPickleSuite @@ -681,10 +647,6 @@ void wrap_StructureAdapter(nb::module_& m) .def("siteCartesianUij", siteCartesianUij_safe, doc_StructureAdapter_siteCartesianUij) - .def("_customPQConfig", - customPQConfig_dispatch, - nb::arg("pqobj"), - doc_StructureAdapter__customPQConfig) .def("diff", structure_diff_shared, nb::arg("other").none(), @@ -698,8 +660,14 @@ void wrap_StructureAdapter(nb::module_& m) nb::class_ nometastructureadapter(m, "NoMetaStructureAdapter", doc_NoMetaStructureAdapter); nometastructureadapter - .def(nb::init(), nb::arg("adapter").none(), - doc_NoMetaStructureAdapter_init) + .def("__init__", [](NoMetaStructureAdapter* self, StructureAdapterPtr source) { + if (PyErr_WarnEx(PyExc_DeprecationWarning, + "NoMetaStructureAdapter is deprecated; use the source " + "adapter directly. Structure metadata no longer " + "configures calculators.", 1) < 0) + nb::raise_python_error(); + new (self) NoMetaStructureAdapter(source); + }, nb::arg("adapter").none(), doc_NoMetaStructureAdapter_init) ; StructureProxyPickleSuite::bind(nometastructureadapter); @@ -734,8 +702,6 @@ void wrap_StructureAdapter(nb::module_& m) const char* doc_StructureAdapter___init__fromstring = nswrap_StructureAdapter::doc_StructureAdapter___init__fromstring; -const char* doc_StructureAdapter__customPQConfig = - nswrap_StructureAdapter::doc_StructureAdapter__customPQConfig; const char* doc_StructureAdapter_diff = nswrap_StructureAdapter::doc_StructureAdapter_diff; diff --git a/tests/test_bondcalculator.py b/tests/test_bondcalculator.py index 508c748..f2496b3 100644 --- a/tests/test_bondcalculator.py +++ b/tests/test_bondcalculator.py @@ -84,15 +84,16 @@ def test_pickling_derived_structure(self): bdc = self.bdc stru0 = DerivedStructureAdapter() + stru0.label = "saved structure" bdc.setStructure(stru0) - self.assertEqual(1, stru0.cpqcount) + self.assertEqual("saved structure", stru0.label) spkl = pickle.dumps(bdc) bdc1 = pickle.loads(spkl) self.assertTrue(stru0 is bdc.getStructure()) stru1 = bdc1.getStructure() self.assertTrue(type(stru1) is DerivedStructureAdapter) self.assertFalse(stru1 is stru0) - self.assertEqual(1, stru1.cpqcount) + self.assertEqual("saved structure", stru1.label) return def test_distances(self): diff --git a/tests/test_bvscalculator.py b/tests/test_bvscalculator.py index 6424dc9..7be0d2b 100644 --- a/tests/test_bvscalculator.py +++ b/tests/test_bvscalculator.py @@ -144,15 +144,16 @@ def test_pickling_derived_structure(self): bvc = self.bvc stru0 = DerivedStructureAdapter() + stru0.label = "saved structure" bvc.setStructure(stru0) - self.assertEqual(1, stru0.cpqcount) + self.assertEqual("saved structure", stru0.label) spkl = pickle.dumps(bvc) bvc1 = pickle.loads(spkl) self.assertTrue(stru0 is bvc.getStructure()) stru1 = bvc1.getStructure() self.assertTrue(type(stru1) is DerivedStructureAdapter) self.assertFalse(stru1 is stru0) - self.assertEqual(1, stru1.cpqcount) + self.assertEqual("saved structure", stru1.label) return def test_table_atom_valence(self): diff --git a/tests/test_calculation_callbacks.py b/tests/test_calculation_callbacks.py index 405845d..25dcea2 100644 --- a/tests/test_calculation_callbacks.py +++ b/tests/test_calculation_callbacks.py @@ -80,36 +80,6 @@ def _finishValue(self): assert calc.calls == 2 -@pytest.mark.parametrize( - "cls", - [ - ext.StructureAdapter, - *[ - ext.AtomicStructureAdapter, - ext.PeriodicStructureAdapter, - ext.CrystalStructureAdapter, - ], - ], -) -def test_custom_config_super_runs_once(cls): - class Derived(cls): - calls = 0 - - def countSites(self): - return 0 - - def _customPQConfig(self, calc): - self.calls += 1 - super()._customPQConfig(calc) - - adapter = Derived() - calc = ext.PairQuantity() - adapter._customPQConfig(calc) - assert adapter.calls == 1 - calc.setStructure(adapter) - assert adapter.calls == 2 - - class CallbackAnisotropyAdapter(ext.StructureAdapter): anisotropy = False diff --git a/tests/test_debyepdfcalculator.py b/tests/test_debyepdfcalculator.py index 1de5481..09709d7 100644 --- a/tests/test_debyepdfcalculator.py +++ b/tests/test_debyepdfcalculator.py @@ -48,6 +48,23 @@ def test___getattr__(self): self.assertEqual(1.23, self.dpdfc.qmin) return + def test_structure_metadata_ignored(self): + """Check that pdffit metadata does not configure the + calculator.""" + bare = self.tio2rutile.copy() + bare.pdffit = {} + meta = self.tio2rutile.copy() + meta.pdffit = dict(scale=2.5, delta1=0.1, delta2=4.0, spdiameter=5.0) + r0, g0 = DebyePDFCalculator(rmax=10)(bare) + r1, g1 = DebyePDFCalculator(rmax=10)(meta) + self.assertTrue(numpy.array_equal(r0, r1)) + self.assertTrue(numpy.array_equal(g0, g1)) + dpdfc = DebyePDFCalculator(scale=3, delta2=0.5) + dpdfc.eval(meta) + self.assertEqual(3, dpdfc.scale) + self.assertEqual(0.5, dpdfc.delta2) + return + def test___setattr__(self): """Check DebyePDFCalculator.__setattr__()""" self.assertNotEqual(1.23, self.dpdfc._getDoubleAttr("rmin")) @@ -196,15 +213,16 @@ def test_pickling_derived_structure(self): dpdfc = self.dpdfc stru0 = DerivedStructureAdapter() + stru0.label = "saved structure" dpdfc.setStructure(stru0) - self.assertEqual(1, stru0.cpqcount) + self.assertEqual("saved structure", stru0.label) spkl = pickle.dumps(dpdfc) dpdfc1 = pickle.loads(spkl) self.assertTrue(stru0 is dpdfc.getStructure()) stru1 = dpdfc1.getStructure() self.assertTrue(type(stru1) is DerivedStructureAdapter) self.assertFalse(stru1 is stru0) - self.assertEqual(1, stru1.cpqcount) + self.assertEqual("saved structure", stru1.label) return diff --git a/tests/test_overlapcalculator.py b/tests/test_overlapcalculator.py index 8c8c8f1..a29a2c8 100644 --- a/tests/test_overlapcalculator.py +++ b/tests/test_overlapcalculator.py @@ -122,15 +122,16 @@ def test_pickling_derived_structure(self): olc = self.olc stru0 = DerivedStructureAdapter() + stru0.label = "saved structure" olc.setStructure(stru0) - self.assertEqual(1, stru0.cpqcount) + self.assertEqual("saved structure", stru0.label) spkl = pickle.dumps(olc) olc1 = pickle.loads(spkl) self.assertTrue(stru0 is olc.getStructure()) stru1 = olc1.getStructure() self.assertTrue(type(stru1) is DerivedStructureAdapter) self.assertFalse(stru1 is stru0) - self.assertEqual(1, stru1.cpqcount) + self.assertEqual("saved structure", stru1.label) return def test_parallel(self): diff --git a/tests/test_pairquantity.py b/tests/test_pairquantity.py index e3c121a..c089cc7 100644 --- a/tests/test_pairquantity.py +++ b/tests/test_pairquantity.py @@ -239,15 +239,16 @@ def test_pickling(self): from testutils import DerivedStructureAdapter stru0 = DerivedStructureAdapter() + stru0.label = "saved structure" self.pq.setStructure(stru0) - self.assertEqual(1, stru0.cpqcount) + self.assertEqual("saved structure", stru0.label) spkl = pickle.dumps(self.pq) pq1 = pickle.loads(spkl) self.assertTrue(stru0 is self.pq.getStructure()) stru1 = pq1.getStructure() self.assertTrue(type(stru1) is DerivedStructureAdapter) self.assertFalse(stru1 is stru0) - self.assertEqual(1, stru1.cpqcount) + self.assertEqual("saved structure", stru1.label) # check pickling of attributes pcnt = PQCounter() pcnt.foo = "asdf" diff --git a/tests/test_pdfcalcobjcryst.py b/tests/test_pdfcalcobjcryst.py index c2dfd82..d93541d 100644 --- a/tests/test_pdfcalcobjcryst.py +++ b/tests/test_pdfcalcobjcryst.py @@ -52,9 +52,6 @@ def _makePDFCalculator(crst, cfgdict): if "type" in cfgdict: pdfc.scatteringfactortable = cfgdict["type"] pdfc.eval(crst) - # avoid metadata override by PDFFitStructure - for k, v in pdfcargs.items(): - setattr(pdfc, k, v) return pdfc diff --git a/tests/test_pdfcalculator.py b/tests/test_pdfcalculator.py index c8e311e..b857233 100644 --- a/tests/test_pdfcalculator.py +++ b/tests/test_pdfcalculator.py @@ -53,17 +53,32 @@ def test___call__(self): r1, g1 = self.pdfcalc(self.tio2rutile, scale=7) self.assertAlmostEqual(7.0, g1[0] / g0[0]) # check application of spdiameter - rutile2 = self.tio2rutile.copy() - # work around Structure bug of shared pdffit dictionary - rutile2.pdffit = dict(self.tio2rutile.pdffit) - rutile2.pdffit["spdiameter"] = 5.0 - r3, g3 = self.pdfcalc(rutile2) + self.pdfcalc.addEnvelope("sphericalshape") + r3, g3 = self.pdfcalc(self.tio2rutile, spdiameter=5.0) self.assertEqual(0.0, sum(g3[r3 >= 5] ** 2)) - r4, g4 = self.pdfcalc(rutile2, scale=1, spdiameter=0) + r4, g4 = self.pdfcalc(self.tio2rutile, scale=1, spdiameter=0) self.assertTrue(numpy.all(r4 == r0)) self.assertTrue(numpy.all(g4 == g0)) return + def test_structure_metadata_ignored(self): + """Check that pdffit metadata does not configure the + calculator.""" + bare = self.tio2rutile.copy() + bare.pdffit = {} + meta = self.tio2rutile.copy() + meta.pdffit = dict(scale=2.5, delta1=0.1, delta2=4.0, spdiameter=5.0) + r0, g0 = PDFCalculator()(bare) + r1, g1 = PDFCalculator()(meta) + self.assertTrue(numpy.array_equal(r0, r1)) + self.assertTrue(numpy.array_equal(g0, g1)) + pdfc = PDFCalculator(scale=3, delta2=0.5) + pdfc(meta) + self.assertEqual(3, pdfc.scale) + self.assertEqual(0.5, pdfc.delta2) + self.assertNotIn("sphericalshape", pdfc.usedenvelopetypes) + return + def test__getDoubleAttr(self): """Check PDFCalculator._getDoubleAttr()""" gdba = self.pdfcalc._getDoubleAttr @@ -113,6 +128,10 @@ def test_eval_rutile(self): dscale = 0.655857 self.pdfcalc.rmin = 1 self.pdfcalc.rmax = 30.0001 + # Apply the historical structure settings explicitly. + self.pdfcalc.delta1 = self.tio2rutile.pdffit["delta1"] + self.pdfcalc.delta2 = self.tio2rutile.pdffit["delta2"] + self.pdfcalc.scale = self.tio2rutile.pdffit["scale"] # apply data scale self.pdfcalc(self.tio2rutile) self.pdfcalc.scale *= dscale @@ -276,15 +295,16 @@ def test_pickling_derived_structure(self): pdfc = self.pdfcalc stru0 = DerivedStructureAdapter() + stru0.label = "saved structure" pdfc.setStructure(stru0) - self.assertEqual(1, stru0.cpqcount) + self.assertEqual("saved structure", stru0.label) spkl = pickle.dumps(pdfc) pdfc1 = pickle.loads(spkl) self.assertTrue(stru0 is pdfc.getStructure()) stru1 = pdfc1.getStructure() self.assertTrue(type(stru1) is DerivedStructureAdapter) self.assertFalse(stru1 is stru0) - self.assertEqual(1, stru1.cpqcount) + self.assertEqual("saved structure", stru1.label) return def test_envelopes(self): diff --git a/tests/test_structureadapter.py b/tests/test_structureadapter.py index 6e3cb1e..bb1bf64 100644 --- a/tests/test_structureadapter.py +++ b/tests/test_structureadapter.py @@ -54,13 +54,8 @@ def test_createStructureAdapter(self): def test_createStructureAdapterTypes(self): """Check types returned by conversion from diffpy.structure.""" - from diffpy.srreal.structureconverters import ( - DiffPyStructureAtomicAdapter, - DiffPyStructurePeriodicAdapter, - ) - adpt = createStructureAdapter(self.nickel) - self.assertTrue(type(adpt) is DiffPyStructurePeriodicAdapter) + self.assertTrue(type(adpt) is PeriodicStructureAdapter) self.nickel.pdffit = None adpt1 = createStructureAdapter(self.nickel) self.assertTrue(type(adpt1) is PeriodicStructureAdapter) @@ -69,7 +64,7 @@ def test_createStructureAdapterTypes(self): self.assertTrue(type(adpt2) is AtomicStructureAdapter) self.nickel.pdffit = dict(scale=1) adpt3 = createStructureAdapter(self.nickel) - self.assertTrue(type(adpt3) is DiffPyStructureAtomicAdapter) + self.assertTrue(type(adpt3) is AtomicStructureAdapter) return def test_createStructureAdapter_int64_occupancy(self): @@ -129,28 +124,13 @@ def setUp(self): self.adpt = self.DerivedCls() return - def test__customPQConfig(self): - """Check if DerivedCls._customPQConfig gets called.""" - self.assertEqual(0, self.adpt.cpqcount) - pc = PDFCalculator() - pc.setStructure(self.adpt) - self.assertEqual(1, self.adpt.cpqcount) - pc(self.adpt) - self.assertEqual(2, self.adpt.cpqcount) - return - def test_pickling(self): """Check pickling of DerivedCls instances.""" - self.adpt.cpqcount = 1 + self.adpt.label = "saved adapter" adpt1 = pickle.loads(pickle.dumps(self.adpt)) self.assertTrue(self.DerivedCls is type(adpt1)) self.assertFalse(self.adpt is adpt1) - self.assertEqual(1, adpt1.cpqcount) - pc = PDFCalculator() - pc.setStructure(adpt1) - self.assertEqual(2, adpt1.cpqcount) - pc(adpt1) - self.assertEqual(3, adpt1.cpqcount) + self.assertEqual("saved adapter", adpt1.label) return @@ -179,48 +159,62 @@ def setUp(self): return def test_nometa(self): - """Check NoMetaStructureAdapter.""" - r0, g0 = PDFCalculator()(self.nickel) - ni1 = self.nickel.copy() - ni1.pdffit["scale"] = 2.0 - r1, g1 = PDFCalculator()(ni1) - self.assertTrue(numpy.array_equal(r0, r1)) - self.assertTrue(numpy.allclose(2 * g0, g1)) - ni1nm = nometa(ni1) - self.assertTrue(ni1nm is nometa(ni1nm)) - r1nm, g1nm = PDFCalculator()(ni1nm) - self.assertTrue(numpy.array_equal(r0, r1nm)) - self.assertTrue(numpy.allclose(g0, g1nm)) - ni2 = self.nickel.copy() - ni2.pdffit["delta2"] = 4 - r2, g2 = PDFCalculator()(ni2) - r2, g2nm = PDFCalculator()(nometa(ni2)) - self.assertFalse(numpy.allclose(g0, g2)) - self.assertTrue(numpy.allclose(g0, g2nm)) - adpt2 = createStructureAdapter(ni2) - ra2, ga2 = PDFCalculator()(adpt2) - ra2, ga2nm = PDFCalculator()(nometa(adpt2)) - self.assertTrue(numpy.allclose(g2, ga2)) - self.assertTrue(numpy.allclose(g0, ga2nm)) - return + """Compatibility conversion creates no metadata proxy.""" + from diffpy.srreal.structureadapter import EMPTY + + adpt = createStructureAdapter(self.nickel) + self.assertIs(adpt, nometa(adpt)) + self.assertIs(EMPTY, nometa(None)) + converted = nometa(self.nickel) + self.assertIs(type(converted), PeriodicStructureAdapter) + self.assertEqual(adpt.countSites(), converted.countSites()) + self.assertIs(converted, nometa(converted)) + for invalid in (77, {}, [1, 2]): + with self.assertRaises(TypeError): + nometa(invalid) def test_nometa_pickling(self): - """Check pickling of the NoMetaStructureAdapter wrapper.""" + """An adapter returned by nometa can still be pickled.""" r0, g0 = PDFCalculator()(self.nickel) - ni1 = self.nickel.copy() - ni1.pdffit["scale"] = 2.0 - ni1nm = pickle.loads(pickle.dumps(nometa(ni1))) - self.assertFalse(ni1nm is ni1) - r1nm, g1nm = PDFCalculator()(ni1nm) - self.assertTrue(numpy.array_equal(r0, r1nm)) - self.assertTrue(numpy.array_equal(g0, g1nm)) - return + self.nickel.pdffit["scale"] = 2.0 + restored = pickle.loads(pickle.dumps(nometa(self.nickel))) + self.assertIs(type(restored), PeriodicStructureAdapter) + r1, g1 = PDFCalculator()(restored) + numpy.testing.assert_array_equal(r0, r1) + numpy.testing.assert_array_equal(g0, g1) def test_nometa_twice(self): """Check that second call of nometa returns the same object.""" adpt1 = nometa(self.nickel) - adpt2 = nometa(adpt1) - self.assertTrue(adpt1 is adpt2) + self.assertIs(adpt1, nometa(adpt1)) + + def test_nometa_proxy_deprecated(self): + """Check that NoMetaStructureAdapter warns and stays usable.""" + from diffpy.srreal.srreal_ext import NoMetaStructureAdapter + + adpt = createStructureAdapter(self.nickel) + with self.assertWarns(DeprecationWarning): + proxy = NoMetaStructureAdapter(adpt) + self.assertEqual(adpt.countSites(), proxy.countSites()) + self.assertIs(proxy, nometa(proxy)) + + def test_legacy_metadata_adapters(self): + """Check that pre-removal adapter classes still unpickle.""" + from diffpy.srreal.structureconverters import ( + DiffPyStructureAtomicAdapter, + DiffPyStructurePeriodicAdapter, + ) + + legacy = (DiffPyStructureAtomicAdapter, DiffPyStructurePeriodicAdapter) + for cls in legacy: + adpt = cls() + adpt.pdffit = {"scale": 25} + adpt1 = pickle.loads(pickle.dumps(adpt)) + self.assertIs(cls, type(adpt1)) + self.assertEqual({"scale": 25}, adpt1.pdffit) + pc = PDFCalculator(scale=3) + pc(adpt1) + self.assertEqual(3, pc.scale) # End of class TestNoMeta @@ -497,11 +491,6 @@ def setUp(self): self.adpt = StructureAdapter() return - # def test__customPQConfig(self): - # """check StructureAdapter._customPQConfig() - # """ - # return - # # def test_countSites(self): # """check StructureAdapter.countSites() # """ diff --git a/tests/testutils.py b/tests/testutils.py index 345f09f..e069edc 100644 --- a/tests/testutils.py +++ b/tests/testutils.py @@ -75,23 +75,6 @@ def _maxNormDiff(yobs, ycalc): # helper class for testing overloading of StructureAdapter -class HasCustomPQConfig(object): - - cpqcount = 0 - - def _customPQConfig(self, pqobj): - self.cpqcount += 1 - return - - -def _installCustomPQConfig(cls): - """Install the test custom-PQ hook without using a mixin base.""" - cls.cpqcount = HasCustomPQConfig.cpqcount - cls._customPQConfig = HasCustomPQConfig._customPQConfig - return cls - - -@_installCustomPQConfig class DerivedStructureAdapter(StructureAdapter): def __init__(self): @@ -145,17 +128,14 @@ def _checkindex(self, idx): # End of class DerivedStructureAdapter -@_installCustomPQConfig class DerivedAtomicStructureAdapter(AtomicStructureAdapter): pass -@_installCustomPQConfig class DerivedPeriodicStructureAdapter(PeriodicStructureAdapter): pass -@_installCustomPQConfig class DerivedCrystalStructureAdapter(CrystalStructureAdapter): pass