Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions news/deprecate-setscattering.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
**Added:**

* <news item>

**Changed:**

* <news item>

**Deprecated:**

* Deprecated setScatteringFactorTableByType.

**Removed:**

* <news item>

**Fixed:**

* <news item>

**Security:**

* <news item>
29 changes: 25 additions & 4 deletions src/extensions/wrap_ScatteringFactorTable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,11 @@ const char* doc_ScatteringFactorTableOwner_setScatteringFactorTableByType = "\
Set internal ScatteringFactorTable according to specified string type.\n\
\n\
tp -- string identifier of a registered ScatteringFactorTable type.\n\
Use ScatteringFactorTable.getRegisteredTypes for the allowed values.\n\
Use ScatteringFactorTable.getRegisteredTypes for the allowed values.\n\
\n\
Deprecated: This method is deprecated and will be removed in the 2.0.0 release.\n\
Use direct assignment to the `scatteringfactortable` property instead, for example:\n\
obj.scatteringfactortable = SFTNeutron()\n\
No return value.\n\
";

Expand Down Expand Up @@ -399,10 +402,28 @@ void wrap_ScatteringFactorTable()
getsftable,
setsftable<ScatteringFactorTableOwner,ScatteringFactorTable>,
doc_ScatteringFactorTableOwner_scatteringfactortable)
// deprecated: prefer assigning the `scatteringfactortable` property
.def("setScatteringFactorTableByType",
&SFTOwner::setScatteringFactorTableByType,
bp::arg("tp"),
doc_ScatteringFactorTableOwner_setScatteringFactorTableByType)
+[](SFTOwner& obj, const std::string& tp)
{
namespace bp = boost::python;
try
{
bp::object warnings = bp::import("warnings");
bp::object builtins = bp::import("builtins");
bp::object DeprecationWarning = builtins.attr("DeprecationWarning");
warnings.attr("warn")(
std::string("setScatteringFactorTableByType is deprecated; "
"assign the 'scatteringfactortable' property directly, for example:\n"
"obj.scatteringfactortable = SFTNeutron()"),
DeprecationWarning,
2);
}
catch (...) { /* don't let warnings break the binding */ }
obj.setScatteringFactorTableByType(tp);
},
bp::arg("tp"),
doc_ScatteringFactorTableOwner_setScatteringFactorTableByType)
.def("getRadiationType",
&SFTOwner::getRadiationType,
return_value_policy<copy_const_reference>(),
Expand Down
24 changes: 23 additions & 1 deletion tests/test_debyepdfcalculator.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@

import pickle
import unittest
import warnings

import numpy
from testutils import _maxNormDiff, loadDiffPyStructure, pickle_with_attr

from diffpy.srreal.pdfcalculator import DebyePDFCalculator, PDFCalculator
from diffpy.srreal.scatteringfactortable import SFTNeutron


##############################################################################
Expand Down Expand Up @@ -141,8 +143,28 @@ def test_partial_pdfs(self):

def test_pickling(self):
"""Check pickling and unpickling of PDFCalculator."""
# New syntax: assign an SFT instance to the property (should not warn)
dpdfc = self.dpdfc
dpdfc.setScatteringFactorTableByType("N")
with warnings.catch_warnings(record=True) as w:
warnings.simplefilter("always")
dpdfc.scatteringfactortable = SFTNeutron()
self.assertFalse(
any(isinstance(x.message, DeprecationWarning) for x in w)
)

dpdfc.scatteringfactortable.setCustomAs("Na", "Na", 7)
spkl = pickle.dumps(dpdfc)
dpdfc1_new = pickle.loads(spkl)
self.assertEqual(
dpdfc.scatteringfactortable.type(),
dpdfc1_new.scatteringfactortable.type(),
)
self.assertEqual(7.0, dpdfc1_new.scatteringfactortable.lookup("Na"))

# Old syntax: call the deprecated method (should warn)
dpdfc = self.dpdfc
with self.assertWarns(DeprecationWarning):
dpdfc.setScatteringFactorTableByType("N")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this test is good. However, do we also need to run a test where this is done using the new way? We want to make sure that

  1. the functionality works with the new syntax (it may be tested elsewhere, but just check)
  2. the functionality works with the old syntax
  3. the old syntax triggers the deprecation warning
  4. the new syntax doesn't trigger the deprecation warning

dpdfc.scatteringfactortable.setCustomAs("Na", "Na", 7)
dpdfc.addEnvelope("sphericalshape")
dpdfc.debyeprecision = 0.001
Expand Down
Loading