Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore deprecation warnings about old-style __init__/__setstate__ constructors in the tests (originally done in #2746) #2759

Merged
merged 3 commits into from
Jan 1, 2021
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
13 changes: 13 additions & 0 deletions tests/pybind11_tests.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#pragma once
#include <pybind11/pybind11.h>
#include <pybind11/eval.h>

#if defined(_MSC_VER) && _MSC_VER < 1910
// We get some really long type names here which causes MSVC 2015 to emit warnings
Expand Down Expand Up @@ -69,3 +70,15 @@ template<> class type_caster<RValueCaster> {
};
PYBIND11_NAMESPACE_END(detail)
PYBIND11_NAMESPACE_END(pybind11)

template <typename F>
void ignoreOldStyleInitWarnings(F &&body) {
py::exec(R"(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Comment from #2746:

  • @YannickJadoul: For reference, this was the old code, without py::exec (and thus without requiring <pybind11/eval.h> to be included in pybind11_tests.h and thus in all tests).

    @bstaletic slightly prefers this [new version], and I think I do to. However, curious to learn what other reviewers think.

    /// Simplified ``with warnigns.catch_warnings()`` wrapper
    template <typename F>
    void ignoreOldStyleInitWarnings(F &&body) {
        auto message = "pybind11-bound class '.+' is using an old-style placement-new '(?:__init__|__setstate__)' which has been deprecated";
        auto category = py::reinterpret_borrow<py::object>(PyExc_FutureWarning);
        auto warnings = py::module_::import("warnings");
        auto context_mgr = warnings.attr("catch_warnings")();
        context_mgr.attr("__enter__")();
        warnings.attr("filterwarnings")("ignore", py::arg("message")=message, py::arg("category")=category);
        body();
        // Exceptions in `body` not handled; see PEP 343 when these would need to be added
        context_mgr.attr("__exit__")(py::none(), py::none(), py::none());
    }

message = "pybind11-bound class '.+' is using an old-style placement-new '(?:__init__|__setstate__)' which has been deprecated"

import warnings
with warnings.catch_warnings():
warnings.filterwarnings("ignore", message=message, category=FutureWarning)
body()
)", py::dict(py::arg("body") = py::cpp_function(body)));
}
49 changes: 30 additions & 19 deletions tests/test_factory_constructors.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -183,11 +183,14 @@ TEST_SUBMODULE(factory_constructors, m) {
auto c4a = [c](pointer_tag, TF4_tag, int a) { (void) c; return new TestFactory4(a);};

// test_init_factory_basic, test_init_factory_casting
py::class_<TestFactory3, std::shared_ptr<TestFactory3>>(m, "TestFactory3")
py::class_<TestFactory3, std::shared_ptr<TestFactory3>> pyTestFactory3(m, "TestFactory3");
pyTestFactory3
.def(py::init([](pointer_tag, int v) { return TestFactoryHelper::construct3(v); }))
.def(py::init([](shared_ptr_tag) { return TestFactoryHelper::construct3(); }))
.def("__init__", [](TestFactory3 &self, std::string v) { new (&self) TestFactory3(v); }) // placement-new ctor

.def(py::init([](shared_ptr_tag) { return TestFactoryHelper::construct3(); }));
ignoreOldStyleInitWarnings([&pyTestFactory3]() {
pyTestFactory3.def("__init__", [](TestFactory3 &self, std::string v) { new (&self) TestFactory3(v); }); // placement-new ctor
});
pyTestFactory3
// factories returning a derived type:
.def(py::init(c4a)) // derived ptr
.def(py::init([](pointer_tag, TF5_tag, int a) { return new TestFactory5(a); }))
Expand Down Expand Up @@ -304,24 +307,32 @@ TEST_SUBMODULE(factory_constructors, m) {
static void operator delete(void *p) { py::print("noisy delete"); ::operator delete(p); }
#endif
};
py::class_<NoisyAlloc>(m, "NoisyAlloc")


py::class_<NoisyAlloc> pyNoisyAlloc(m, "NoisyAlloc");
// Since these overloads have the same number of arguments, the dispatcher will try each of
// them until the arguments convert. Thus we can get a pre-allocation here when passing a
// single non-integer:
.def("__init__", [](NoisyAlloc *a, int i) { new (a) NoisyAlloc(i); }) // Regular constructor, runs first, requires preallocation
.def(py::init([](double d) { return new NoisyAlloc(d); }))

// The two-argument version: first the factory pointer overload.
.def(py::init([](int i, int) { return new NoisyAlloc(i); }))
// Return-by-value:
.def(py::init([](double d, int) { return NoisyAlloc(d); }))
// Old-style placement new init; requires preallocation
.def("__init__", [](NoisyAlloc &a, double d, double) { new (&a) NoisyAlloc(d); })
// Requires deallocation of previous overload preallocated value:
.def(py::init([](int i, double) { return new NoisyAlloc(i); }))
// Regular again: requires yet another preallocation
.def("__init__", [](NoisyAlloc &a, int i, std::string) { new (&a) NoisyAlloc(i); })
;
ignoreOldStyleInitWarnings([&pyNoisyAlloc]() {
pyNoisyAlloc.def("__init__", [](NoisyAlloc *a, int i) { new (a) NoisyAlloc(i); }); // Regular constructor, runs first, requires preallocation
});

pyNoisyAlloc.def(py::init([](double d) { return new NoisyAlloc(d); }));

// The two-argument version: first the factory pointer overload.
pyNoisyAlloc.def(py::init([](int i, int) { return new NoisyAlloc(i); }));
// Return-by-value:
pyNoisyAlloc.def(py::init([](double d, int) { return NoisyAlloc(d); }));
// Old-style placement new init; requires preallocation
ignoreOldStyleInitWarnings([&pyNoisyAlloc]() {
pyNoisyAlloc.def("__init__", [](NoisyAlloc &a, double d, double) { new (&a) NoisyAlloc(d); });
});
// Requires deallocation of previous overload preallocated value:
pyNoisyAlloc.def(py::init([](int i, double) { return new NoisyAlloc(i); }));
// Regular again: requires yet another preallocation
ignoreOldStyleInitWarnings([&pyNoisyAlloc]() {
pyNoisyAlloc.def("__init__", [](NoisyAlloc &a, int i, std::string) { new (&a) NoisyAlloc(i); });
});



Expand Down
58 changes: 33 additions & 25 deletions tests/test_pickling.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ TEST_SUBMODULE(pickling, m) {
using Pickleable::Pickleable;
};

py::class_<Pickleable>(m, "Pickleable")
py::class_<Pickleable> pyPickleable(m, "Pickleable");
pyPickleable
.def(py::init<std::string>())
.def("value", &Pickleable::value)
.def("extra1", &Pickleable::extra1)
Expand All @@ -43,17 +44,20 @@ TEST_SUBMODULE(pickling, m) {
.def("__getstate__", [](const Pickleable &p) {
/* Return a tuple that fully encodes the state of the object */
return py::make_tuple(p.value(), p.extra1(), p.extra2());
})
.def("__setstate__", [](Pickleable &p, py::tuple t) {
if (t.size() != 3)
throw std::runtime_error("Invalid state!");
/* Invoke the constructor (need to use in-place version) */
new (&p) Pickleable(t[0].cast<std::string>());

/* Assign any additional state */
p.setExtra1(t[1].cast<int>());
p.setExtra2(t[2].cast<int>());
});
ignoreOldStyleInitWarnings([&pyPickleable]() {
pyPickleable
.def("__setstate__", [](Pickleable &p, py::tuple t) {
if (t.size() != 3)
throw std::runtime_error("Invalid state!");
/* Invoke the constructor (need to use in-place version) */
new (&p) Pickleable(t[0].cast<std::string>());

/* Assign any additional state */
p.setExtra1(t[1].cast<int>());
p.setExtra2(t[2].cast<int>());
});
});

py::class_<PickleableNew, Pickleable>(m, "PickleableNew")
.def(py::init<std::string>())
Expand Down Expand Up @@ -87,27 +91,31 @@ TEST_SUBMODULE(pickling, m) {
using PickleableWithDict::PickleableWithDict;
};

py::class_<PickleableWithDict>(m, "PickleableWithDict", py::dynamic_attr())
py::class_<PickleableWithDict> pyPickleableWithDict(m, "PickleableWithDict", py::dynamic_attr());
pyPickleableWithDict
.def(py::init<std::string>())
.def_readwrite("value", &PickleableWithDict::value)
.def_readwrite("extra", &PickleableWithDict::extra)
.def("__getstate__", [](py::object self) {
/* Also include __dict__ in state */
return py::make_tuple(self.attr("value"), self.attr("extra"), self.attr("__dict__"));
})
.def("__setstate__", [](py::object self, py::tuple t) {
if (t.size() != 3)
throw std::runtime_error("Invalid state!");
/* Cast and construct */
auto& p = self.cast<PickleableWithDict&>();
new (&p) PickleableWithDict(t[0].cast<std::string>());

/* Assign C++ state */
p.extra = t[1].cast<int>();

/* Assign Python state */
self.attr("__dict__") = t[2];
});
ignoreOldStyleInitWarnings([&pyPickleableWithDict]() {
pyPickleableWithDict
.def("__setstate__", [](py::object self, py::tuple t) {
if (t.size() != 3)
throw std::runtime_error("Invalid state!");
/* Cast and construct */
auto& p = self.cast<PickleableWithDict&>();
new (&p) PickleableWithDict(t[0].cast<std::string>());

/* Assign C++ state */
p.extra = t[1].cast<int>();

/* Assign Python state */
self.attr("__dict__") = t[2];
});
});

py::class_<PickleableWithDictNew, PickleableWithDict>(m, "PickleableWithDictNew")
.def(py::init<std::string>())
Expand Down