Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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>
28 changes: 24 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 a future 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,27 @@ 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, use SFTNeutron()/SFTXray())."),
Copy link
Contributor

Choose a reason for hiding this comment

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

let's give an example of the exact syntax in this help message, as you did in the comment above. But this will be more visible to users.

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
3 changes: 2 additions & 1 deletion tests/test_debyepdfcalculator.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,8 @@ def test_partial_pdfs(self):
def test_pickling(self):
"""Check pickling and unpickling of PDFCalculator."""
dpdfc = self.dpdfc
dpdfc.setScatteringFactorTableByType("N")
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