From 7716633d1f90f6ee2ef4324a23f8242a77c19225 Mon Sep 17 00:00:00 2001 From: Gerry Manoim Date: Tue, 30 Jun 2020 18:17:07 -0400 Subject: [PATCH 1/8] MAINT: Move sparsehash code to library_wrappers/sparsehash --- Makefile | 2 +- .../sparsehash.h} | 0 .../test_sparsehash.cc} | 12 ++++++++++++ tests/test_object_map_key.cc | 3 --- tests/test_to_object.cc | 6 +----- 5 files changed, 14 insertions(+), 9 deletions(-) rename include/libpy/{dense_hash_map.h => library_wrappers/sparsehash.h} (100%) rename tests/{test_dense_hash_map.cc => library_wrappers/test_sparsehash.cc} (82%) diff --git a/Makefile b/Makefile index a9f5c43f..e7a5d1f5 100644 --- a/Makefile +++ b/Makefile @@ -150,7 +150,7 @@ GTEST_SRCS := $(wildcard $(GTEST_DIR)/src/*.cc) \ $(wildcard $(GTEST_DIR)/src/*.h) $(GTEST_HEADERS) GTEST_FILTER ?= '*' -TEST_SOURCES := $(wildcard tests/*.cc) +TEST_SOURCES := $(wildcard tests/*.cc) $(wildcard tests/library_wrappers/*.cc) TEST_DFILES := $(TEST_SOURCES:.cc=.d) TEST_OBJECTS := $(TEST_SOURCES:.cc=.o) TEST_HEADERS := $(wildcard tests/*.h) $(GTEST_HEADERS) diff --git a/include/libpy/dense_hash_map.h b/include/libpy/library_wrappers/sparsehash.h similarity index 100% rename from include/libpy/dense_hash_map.h rename to include/libpy/library_wrappers/sparsehash.h diff --git a/tests/test_dense_hash_map.cc b/tests/library_wrappers/test_sparsehash.cc similarity index 82% rename from tests/test_dense_hash_map.cc rename to tests/library_wrappers/test_sparsehash.cc index afd235b2..c4f8b8c4 100644 --- a/tests/test_dense_hash_map.cc +++ b/tests/library_wrappers/test_sparsehash.cc @@ -42,4 +42,16 @@ TEST(dense_hash_set, invalid_empty_key) { EXPECT_THROW((M8_key{py::datetime64ns::nat()}), std::invalid_argument); EXPECT_THROW((M8_key{py::datetime64ns::nat(), 10}), std::invalid_argument); } + +TEST(any_vtable, map_key) { + py::dense_hash_map map(py::any_vtable{}); + + map[py::any_vtable::make()] = 0; + map[py::any_vtable::make()] = 1; + map[py::any_vtable::make()] = 2; + + EXPECT_EQ(map[py::any_vtable::make()], 0); + EXPECT_EQ(map[py::any_vtable::make()], 1); + EXPECT_EQ(map[py::any_vtable::make()], 2); +} } // namespace test_dense_hash_map diff --git a/tests/test_object_map_key.cc b/tests/test_object_map_key.cc index 824b4993..9c44779d 100644 --- a/tests/test_object_map_key.cc +++ b/tests/test_object_map_key.cc @@ -3,7 +3,6 @@ #include "gtest/gtest.h" -#include "libpy/dense_hash_map.h" #include "libpy/exception.h" #include "libpy/meta.h" #include "libpy/object_map_key.h" @@ -241,8 +240,6 @@ void test_use_in_map(M map) { TEST_F(object_map_key, use_in_map) { test_use_in_map(std::map{}); test_use_in_map(std::unordered_map{}); - test_use_in_map(py::sparse_hash_map{}); - test_use_in_map(py::dense_hash_map{py::object_map_key{}}); } TEST_F(object_map_key, convert) { diff --git a/tests/test_to_object.cc b/tests/test_to_object.cc index 09bbbf81..3323d883 100644 --- a/tests/test_to_object.cc +++ b/tests/test_to_object.cc @@ -6,7 +6,6 @@ #include "libpy/any.h" #include "libpy/char_sequence.h" -#include "libpy/dense_hash_map.h" #include "libpy/itertools.h" #include "libpy/meta.h" #include "libpy/numpy_utils.h" @@ -114,10 +113,7 @@ TEST_F(to_object, map_to_object) { // NOTE: This test takes a long time to compile (about a .5s per entry in this // tuple). This is just enough coverage to test all three of our hash table types, // and a few important key/value types. - auto maps = std::make_tuple(py::dense_hash_map>( - "missing_value"s), - py::sparse_hash_map>(), - std::unordered_map()); + auto maps = std::make_tuple(std::unordered_map()); // Call test_map_to_object_impl on each entry in ``maps``. std::apply([&](auto... map) { (test_map_to_object_impl(map), ...); }, maps); From 400815f5525763658137eef38229b03df90cdd1d Mon Sep 17 00:00:00 2001 From: Gerry Manoim Date: Tue, 30 Jun 2020 18:34:49 -0400 Subject: [PATCH 2/8] TST: fix up tests --- tests/library_wrappers/test_sparsehash.cc | 4 +++- tests/test_any.cc | 13 ------------- 2 files changed, 3 insertions(+), 14 deletions(-) diff --git a/tests/library_wrappers/test_sparsehash.cc b/tests/library_wrappers/test_sparsehash.cc index c4f8b8c4..4873172b 100644 --- a/tests/library_wrappers/test_sparsehash.cc +++ b/tests/library_wrappers/test_sparsehash.cc @@ -3,8 +3,9 @@ #include "gtest/gtest.h" +#include "libpy/any.h" #include "libpy/datetime64.h" -#include "libpy/dense_hash_map.h" +#include "libpy/library_wrappers/sparsehash.h" namespace test_dense_hash_map { TEST(dense_hash_map, invalid_empty_key) { @@ -54,4 +55,5 @@ TEST(any_vtable, map_key) { EXPECT_EQ(map[py::any_vtable::make()], 1); EXPECT_EQ(map[py::any_vtable::make()], 2); } + } // namespace test_dense_hash_map diff --git a/tests/test_any.cc b/tests/test_any.cc index d2ad61b5..1deb7167 100644 --- a/tests/test_any.cc +++ b/tests/test_any.cc @@ -4,7 +4,6 @@ #include "gtest/gtest.h" #include "libpy/any.h" -#include "libpy/dense_hash_map.h" namespace test_any { TEST(any_vtable, void_vtable) { @@ -87,18 +86,6 @@ TEST(any_vtable, ostream_format) { } } -TEST(any_vtable, map_key) { - py::dense_hash_map map(py::any_vtable{}); - - map[py::any_vtable::make()] = 0; - map[py::any_vtable::make()] = 1; - map[py::any_vtable::make()] = 2; - - EXPECT_EQ(map[py::any_vtable::make()], 0); - EXPECT_EQ(map[py::any_vtable::make()], 1); - EXPECT_EQ(map[py::any_vtable::make()], 2); -} - TEST(any_ref, test_construction) { int underlying = 1; py::any_ref ref = py::make_any_ref(underlying); From 7f5a12893968a128bcd26838fe60965b756a7259 Mon Sep 17 00:00:00 2001 From: Gerry Manoim Date: Thu, 2 Jul 2020 14:39:31 -0400 Subject: [PATCH 3/8] move test back to test_any --- tests/library_wrappers/test_sparsehash.cc | 12 ------------ tests/test_any.cc | 12 ++++++++++++ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/library_wrappers/test_sparsehash.cc b/tests/library_wrappers/test_sparsehash.cc index 4873172b..0c2b0951 100644 --- a/tests/library_wrappers/test_sparsehash.cc +++ b/tests/library_wrappers/test_sparsehash.cc @@ -44,16 +44,4 @@ TEST(dense_hash_set, invalid_empty_key) { EXPECT_THROW((M8_key{py::datetime64ns::nat(), 10}), std::invalid_argument); } -TEST(any_vtable, map_key) { - py::dense_hash_map map(py::any_vtable{}); - - map[py::any_vtable::make()] = 0; - map[py::any_vtable::make()] = 1; - map[py::any_vtable::make()] = 2; - - EXPECT_EQ(map[py::any_vtable::make()], 0); - EXPECT_EQ(map[py::any_vtable::make()], 1); - EXPECT_EQ(map[py::any_vtable::make()], 2); -} - } // namespace test_dense_hash_map diff --git a/tests/test_any.cc b/tests/test_any.cc index 1deb7167..41aed8d9 100644 --- a/tests/test_any.cc +++ b/tests/test_any.cc @@ -86,6 +86,18 @@ TEST(any_vtable, ostream_format) { } } +TEST(any_vtable, map_key) { + std::unordered_map map; + + map[py::any_vtable::make()] = 0; + map[py::any_vtable::make()] = 1; + map[py::any_vtable::make()] = 2; + + EXPECT_EQ(map[py::any_vtable::make()], 0); + EXPECT_EQ(map[py::any_vtable::make()], 1); + EXPECT_EQ(map[py::any_vtable::make()], 2); +} + TEST(any_ref, test_construction) { int underlying = 1; py::any_ref ref = py::make_any_ref(underlying); From 54ad0bc85ffd2ef81e73d1775954500bcfb47c57 Mon Sep 17 00:00:00 2001 From: Gerry Manoim Date: Thu, 2 Jul 2020 14:40:38 -0400 Subject: [PATCH 4/8] add wrappers to test headers --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index e7a5d1f5..839b3bb8 100644 --- a/Makefile +++ b/Makefile @@ -153,7 +153,7 @@ GTEST_FILTER ?= '*' TEST_SOURCES := $(wildcard tests/*.cc) $(wildcard tests/library_wrappers/*.cc) TEST_DFILES := $(TEST_SOURCES:.cc=.d) TEST_OBJECTS := $(TEST_SOURCES:.cc=.o) -TEST_HEADERS := $(wildcard tests/*.h) $(GTEST_HEADERS) +TEST_HEADERS := $(wildcard tests/*.h) $(wildcard tests/library_wrappers/*.h) $(GTEST_HEADERS) TEST_INCLUDE := -I tests -I $(GTEST_DIR)/include TEST_MODULE := tests/_runner$(SO_SUFFIX) PYTHON_TESTS := $(wildcard tests/*.py) From 95c02ce48a87baea5cbca78710bf235d1fec3a53 Mon Sep 17 00:00:00 2001 From: Gerry Manoim Date: Thu, 2 Jul 2020 17:53:40 -0400 Subject: [PATCH 5/8] move test utils, sparsehash wrappers, added tests for to_object wrappers, new set tester --- include/libpy/library_wrappers/sparsehash.h | 14 ++ tests/library_wrappers/test_sparsehash.cc | 35 +++- tests/test_to_object.cc | 146 +---------------- tests/test_utils.h | 172 ++++++++++++++++++++ 4 files changed, 227 insertions(+), 140 deletions(-) diff --git a/include/libpy/library_wrappers/sparsehash.h b/include/libpy/library_wrappers/sparsehash.h index add4793b..3de4eec9 100644 --- a/include/libpy/library_wrappers/sparsehash.h +++ b/include/libpy/library_wrappers/sparsehash.h @@ -153,5 +153,19 @@ template struct to_object> : public set_to_object> {}; + +template +struct to_object> + : public map_to_object> {}; + +template +struct to_object> + : public map_to_object> {}; + +template +struct to_object> + : public set_to_object> {}; + + } // namespace dispatch } // namespace py diff --git a/tests/library_wrappers/test_sparsehash.cc b/tests/library_wrappers/test_sparsehash.cc index 0c2b0951..34ee75b1 100644 --- a/tests/library_wrappers/test_sparsehash.cc +++ b/tests/library_wrappers/test_sparsehash.cc @@ -3,11 +3,40 @@ #include "gtest/gtest.h" -#include "libpy/any.h" #include "libpy/datetime64.h" +#include "libpy/itertools.h" #include "libpy/library_wrappers/sparsehash.h" -namespace test_dense_hash_map { +#include "test_utils.h" + +namespace test_sparsehash { + +using namespace std::literals; +using namespace py::cs::literals; + +class sparsehash_to_object : public with_python_interpreter {}; + + +TEST_F(sparsehash_to_object, map_to_object) { + // NOTE: This test takes a long time to compile (about a .5s per entry in this + // tuple). This is just enough coverage to test all three of our hash table types, + // and a few important key/value types. + auto a = google::sparse_hash_map(); + auto b = google::dense_hash_map(); + b.set_empty_key("the_empty_key"s); + + auto maps = std::make_tuple(a, b); + + // Call test_map_to_object_impl on each entry in ``maps``. + std::apply([&](auto... map) { (py_test::test_map_to_object_impl(map), ...); }, maps); +} + +TEST_F(sparsehash_to_object, set_to_object) { + auto filler = py_test::examples(); + auto a = google::dense_hash_set(filler.begin(), filler.end(), "the_empty_key"s); + py_test::test_set_to_object_impl(a); +} + TEST(dense_hash_map, invalid_empty_key) { using double_key = py::dense_hash_map; EXPECT_THROW((double_key{std::numeric_limits::quiet_NaN()}), @@ -44,4 +73,4 @@ TEST(dense_hash_set, invalid_empty_key) { EXPECT_THROW((M8_key{py::datetime64ns::nat(), 10}), std::invalid_argument); } -} // namespace test_dense_hash_map +} // namespace test_sparsehash diff --git a/tests/test_to_object.cc b/tests/test_to_object.cc index 3323d883..2c1a514b 100644 --- a/tests/test_to_object.cc +++ b/tests/test_to_object.cc @@ -20,95 +20,6 @@ using namespace py::cs::literals; class to_object : public with_python_interpreter {}; -template -std::array examples(); - -template<> -std::array examples() { - return {-200, 0, 1000}; -} - -template<> -std::array examples() { - return {"foo", "", "arglebargle"}; -} - -template<> -std::array, 3> examples() { - std::array foo{'f', 'o', 'o'}; - std::array bar{'b', 'a', 'r'}; - std::array baz{'b', 'a', 'z'}; - return {foo, bar, baz}; -} - -template<> -std::array examples() { - return {true, false, true}; -} - -template<> -std::array examples() { - return {-1.0, -0.0, 100.0}; -} - -template<> -std::array, 3> examples() { - Py_INCREF(Py_True); - Py_INCREF(Py_False); - Py_INCREF(Py_None); - return {py::owned_ref<>(Py_True), - py::owned_ref<>(Py_False), - py::owned_ref<>(Py_None)}; -} - -template -void test_map_to_object_impl(M m) { - - // Fill the map with some example values. - auto it = py::zip(examples(), - examples()); - for (auto [key, value] : it) { - m[key] = value; - } - - auto check_python_map = [&](py::owned_ref ob) { - ASSERT_TRUE(ob) << "to_object should not return null"; - EXPECT_TRUE(PyDict_Check(ob.get())); - - // Python map should be the same length as C++ map. - Py_ssize_t len = PyDict_Size(ob.get()); - EXPECT_EQ(std::size_t(len), m.size()) - << "Python dict length should match C++ map length."; - - // Key/Value pairs in the python map should match the result of calling - // to_object on each key/value pair in the C++ map. - for (auto& [cxx_key, cxx_value] : m) { - auto py_key = py::to_object(cxx_key); - auto py_value = py::to_object(cxx_value); - - py::borrowed_ref result = PyDict_GetItem(ob.get(), py_key.get()); - ASSERT_TRUE(result) << "Key should have been in the map"; - - bool values_equal = - PyObject_RichCompareBool(py_value.get(), result.get(), Py_EQ); - EXPECT_EQ(values_equal, 1) << "Dict values were not equal"; - } - }; - - // Check to_object with value, const value, and rvalue reference. - - py::owned_ref result = py::to_object(m); - check_python_map(result); - - const M& const_ref = m; - py::owned_ref constref_result = py::to_object(const_ref); - check_python_map(constref_result); - - M copy = m; // Make a copy before moving b/c the lambda above uses ``m``. - py::owned_ref rvalueref_result = py::to_object(std::move(copy)); - check_python_map(rvalueref_result); -} - TEST_F(to_object, map_to_object) { // NOTE: This test takes a long time to compile (about a .5s per entry in this // tuple). This is just enough coverage to test all three of our hash table types, @@ -116,63 +27,24 @@ TEST_F(to_object, map_to_object) { auto maps = std::make_tuple(std::unordered_map()); // Call test_map_to_object_impl on each entry in ``maps``. - std::apply([&](auto... map) { (test_map_to_object_impl(map), ...); }, maps); -} - -template -void test_sequence_to_object_impl(V v) { - auto check_python_list = [&](py::owned_ref ob) { - ASSERT_TRUE(ob) << "to_object should not return null"; - EXPECT_EQ(PyList_Check(ob.get()), 1) << "ob should be a list"; - - Py_ssize_t len = PyList_GET_SIZE(ob.get()); - EXPECT_EQ(std::size_t(len), v.size()) - << "Python list length should match C++ vector length."; - - // Values in Python list should be the result of calling to_object on each entry - // in the C++ vector. - for (auto [i, cxx_value] : py::enumerate(v)) { - auto py_value = py::to_object(cxx_value); - - py::borrowed_ref result = PyList_GetItem(ob.get(), i); - ASSERT_TRUE(result) << "Should have had a value at index " << i; - - bool values_equal = - PyObject_RichCompareBool(py_value.get(), result.get(), Py_EQ); - EXPECT_EQ(values_equal, 1) - << "List values at index " << i << " were not equal"; - } - }; - - // Check to_object with value, const value, and rvalue reference. - - py::owned_ref result = py::to_object(v); - check_python_list(result); - - const V& const_ref = v; - py::owned_ref constref_result = py::to_object(const_ref); - check_python_list(constref_result); - - V copy = v; // Make a copy before moving b/c the lambda above uses ``v``. - py::owned_ref rvalueref_result = py::to_object(std::move(copy)); - check_python_list(rvalueref_result); + std::apply([&](auto... map) { (py_test::test_map_to_object_impl(map), ...); }, maps); } TEST_F(to_object, vector_to_object) { auto to_vec = [](const auto& arr) { return std::vector(arr.begin(), arr.end()); }; - auto vectors = std::make_tuple(to_vec(examples()), - to_vec(examples()), - to_vec(examples>())); + auto vectors = std::make_tuple(to_vec(py_test::examples()), + to_vec(py_test::examples()), + to_vec(py_test::examples>())); // Call test_sequence_to_object_impl on each entry in `vectors`. - std::apply([&](auto... vec) { (test_sequence_to_object_impl(vec), ...); }, vectors); + std::apply([&](auto... vec) { (py_test::test_sequence_to_object_impl(vec), ...); }, vectors); } TEST_F(to_object, array_to_object) { - auto arrays = std::make_tuple(examples(), - examples(), - examples>()); + auto arrays = std::make_tuple(py_test::examples(), + py_test::examples(), + py_test::examples>()); // Call test_sequence_to_object_impl on each entry in `arrays`. - std::apply([&](auto... arr) { (test_sequence_to_object_impl(arr), ...); }, arrays); + std::apply([&](auto... arr) { (py_test::test_sequence_to_object_impl(arr), ...); }, arrays); } template diff --git a/tests/test_utils.h b/tests/test_utils.h index f6bb33c7..3aee9abd 100644 --- a/tests/test_utils.h +++ b/tests/test_utils.h @@ -8,6 +8,7 @@ #include "libpy/call_function.h" #include "libpy/detail/python.h" #include "libpy/exception.h" +#include "libpy/itertools.h" #include "libpy/owned_ref.h" #include "libpy/util.h" @@ -187,3 +188,174 @@ inline py::owned_ref<> run_python( */ #define EVAL_PYTHON(python_source, ...) \ ::detail::run_python(python_source, __FILE__, __LINE__, true, ##__VA_ARGS__) + +namespace py_test { + +template +inline std::array examples(); + +template<> +inline std::array examples() { + return {-200, 0, 1000}; +} + +template<> +inline std::array examples() { + return {"foo", "", "arglebargle"}; +} + +template<> +inline std::array, 3> examples() { + std::array foo{'f', 'o', 'o'}; + std::array bar{'b', 'a', 'r'}; + std::array baz{'b', 'a', 'z'}; + return {foo, bar, baz}; +} + +template<> +inline std::array examples() { + return {true, false, true}; +} + +template<> +inline std::array examples() { + return {-1.0, -0.0, 100.0}; +} + +template<> +inline std::array, 3> examples() { + Py_INCREF(Py_True); + Py_INCREF(Py_False); + Py_INCREF(Py_None); + return {py::owned_ref<>(Py_True), + py::owned_ref<>(Py_False), + py::owned_ref<>(Py_None)}; +} + +template +inline void test_map_to_object_impl(M m) { + + // Fill the map with some example values. + auto it = py::zip(examples(), + examples()); + for (auto [key, value] : it) { + m[key] = value; + } + + auto check_python_map = [&](py::owned_ref ob) { + ASSERT_TRUE(ob) << "to_object should not return null"; + EXPECT_TRUE(PyDict_Check(ob.get())); + + // Python map should be the same length as C++ map. + Py_ssize_t len = PyDict_Size(ob.get()); + EXPECT_EQ(std::size_t(len), m.size()) + << "Python dict length should match C++ map length."; + + // Key/Value pairs in the python map should match the result of calling + // to_object on each key/value pair in the C++ map. + for (auto& [cxx_key, cxx_value] : m) { + auto py_key = py::to_object(cxx_key); + auto py_value = py::to_object(cxx_value); + + py::borrowed_ref result = PyDict_GetItem(ob.get(), py_key.get()); + ASSERT_TRUE(result) << "Key should have been in the map"; + + bool values_equal = + PyObject_RichCompareBool(py_value.get(), result.get(), Py_EQ); + EXPECT_EQ(values_equal, 1) << "Dict values were not equal"; + } + }; + + // Check to_object with value, const value, and rvalue reference. + + py::owned_ref result = py::to_object(m); + check_python_map(result); + + const M& const_ref = m; + py::owned_ref constref_result = py::to_object(const_ref); + check_python_map(constref_result); + + M copy = m; // Make a copy before moving b/c the lambda above uses ``m``. + py::owned_ref rvalueref_result = py::to_object(std::move(copy)); + check_python_map(rvalueref_result); +} + +template +inline void test_sequence_to_object_impl(V v) { + auto check_python_list = [&](py::owned_ref ob) { + ASSERT_TRUE(ob) << "to_object should not return null"; + EXPECT_EQ(PyList_Check(ob.get()), 1) << "ob should be a list"; + + Py_ssize_t len = PyList_GET_SIZE(ob.get()); + EXPECT_EQ(std::size_t(len), v.size()) + << "Python list length should match C++ vector length."; + + // Values in Python list should be the result of calling to_object on each entry + // in the C++ vector. + for (auto [i, cxx_value] : py::enumerate(v)) { + auto py_value = py::to_object(cxx_value); + + py::borrowed_ref result = PyList_GetItem(ob.get(), i); + ASSERT_TRUE(result) << "Should have had a value at index " << i; + + bool values_equal = + PyObject_RichCompareBool(py_value.get(), result.get(), Py_EQ); + EXPECT_EQ(values_equal, 1) + << "List values at index " << i << " were not equal"; + } + }; + + // Check to_object with value, const value, and rvalue reference. + + py::owned_ref result = py::to_object(v); + check_python_list(result); + + const V& const_ref = v; + py::owned_ref constref_result = py::to_object(const_ref); + check_python_list(constref_result); + + V copy = v; // Make a copy before moving b/c the lambda above uses ``v``. + py::owned_ref rvalueref_result = py::to_object(std::move(copy)); + check_python_list(rvalueref_result); +} + +template +inline void test_set_to_object_impl(V v) { + auto check_python_set = [&](py::owned_ref ob) { + ASSERT_TRUE(ob) << "to_object should not return null"; + EXPECT_EQ(PySet_Check(ob.get()), 1) << "ob should be a set"; + + Py_ssize_t len = PySet_GET_SIZE(ob.get()); + EXPECT_EQ(std::size_t(len), v.size()) + << "Python set length should match C++ set length."; + + // Values in Python list should be the result of calling to_object on each entry + // in the C++ vector. + for (auto cxx_value : v) { + auto py_value = py::to_object(cxx_value); + + auto result = PySet_Contains(ob.get(), py_value.get()); + ASSERT_TRUE(result) << "Should contain value " << cxx_value; + + // bool values_equal = + // PyObject_RichCompareBool(py_value.get(), result.get(), Py_EQ); + // EXPECT_EQ(values_equal, 1) + // << "List values at index " << i << " were not equal"; + } + }; + + // Check to_object with value, const value, and rvalue reference. + + py::owned_ref result = py::to_object(v); + check_python_set(result); + + const V& const_ref = v; + py::owned_ref constref_result = py::to_object(const_ref); + check_python_set(constref_result); + + V copy = v; // Make a copy before moving b/c the lambda above uses ``v``. + py::owned_ref rvalueref_result = py::to_object(std::move(copy)); + check_python_set(rvalueref_result); +} + +} From 520f7434433afcb890187c7e1f91c1235dfbb008 Mon Sep 17 00:00:00 2001 From: Gerry Manoim Date: Thu, 2 Jul 2020 18:23:47 -0400 Subject: [PATCH 6/8] pr feedback --- README.rst | 20 ++++------------- docs/source/install.rst | 21 ++++-------------- include/libpy/library_wrappers/sparsehash.h | 6 ++++++ tests/library_wrappers/test_sparsehash.cc | 24 ++++++++++++++------- tests/test_to_object.cc | 6 ++---- tests/test_utils.h | 8 +++---- 6 files changed, 36 insertions(+), 49 deletions(-) diff --git a/README.rst b/README.rst index 32604a1a..6fa83036 100644 --- a/README.rst +++ b/README.rst @@ -28,25 +28,13 @@ libpy requires: - gcc>=8 or clang>=10 - numpy>=1.11.3 -libpy also depends on the following system packages: +Optional Requirements +--------------------- -- google sparsehash - -To install these dependencies: - -ubuntu -~~~~~~ - -.. code-block:: bash +libpy optionally provides wrappers for the following libraries: - $ sudo apt install libsparsehash-dev - -macOS -~~~~~ - -.. code-block:: bash +- google sparsehash - $ brew install google-sparsehash Install ------- diff --git a/docs/source/install.rst b/docs/source/install.rst index ddc89f21..0381e361 100644 --- a/docs/source/install.rst +++ b/docs/source/install.rst @@ -14,25 +14,12 @@ lipy requires: - gcc>=8 or clang>=10 - numpy>=1.11.3 -libpy also depends on the following system packages: +Optional Requirements +--------------------- -- google sparsehash - -To install these dependencies: - -ubuntu -~~~~~~ - -.. code-block:: bash +libpy optionally provides wrappers for the following libraries: - $ sudo apt install libsparsehash-dev - -macOS -~~~~~ - -.. code-block:: bash - - $ brew install google-sparsehash +- google sparsehash Install ------- diff --git a/include/libpy/library_wrappers/sparsehash.h b/include/libpy/library_wrappers/sparsehash.h index 3de4eec9..14c5e458 100644 --- a/include/libpy/library_wrappers/sparsehash.h +++ b/include/libpy/library_wrappers/sparsehash.h @@ -5,6 +5,8 @@ #include #include #include +#include + #include "libpy/to_object.h" @@ -166,6 +168,10 @@ template struct to_object> : public set_to_object> {}; +template +struct to_object> + : public set_to_object> {}; + } // namespace dispatch } // namespace py diff --git a/tests/library_wrappers/test_sparsehash.cc b/tests/library_wrappers/test_sparsehash.cc index 34ee75b1..11ae6770 100644 --- a/tests/library_wrappers/test_sparsehash.cc +++ b/tests/library_wrappers/test_sparsehash.cc @@ -17,21 +17,29 @@ using namespace py::cs::literals; class sparsehash_to_object : public with_python_interpreter {}; -TEST_F(sparsehash_to_object, map_to_object) { +TEST_F(sparsehash_to_object, sparse_hash_map) { // NOTE: This test takes a long time to compile (about a .5s per entry in this // tuple). This is just enough coverage to test all three of our hash table types, // and a few important key/value types. - auto a = google::sparse_hash_map(); - auto b = google::dense_hash_map(); - b.set_empty_key("the_empty_key"s); + auto map = google::sparse_hash_map(); - auto maps = std::make_tuple(a, b); + py_test::test_map_to_object_impl(map); +} + +TEST_F(sparsehash_to_object, dense_hash_map) { + auto map = google::dense_hash_map(); + map.set_empty_key("the_empty_key"s); + + py_test::test_map_to_object_impl(map); +} - // Call test_map_to_object_impl on each entry in ``maps``. - std::apply([&](auto... map) { (py_test::test_map_to_object_impl(map), ...); }, maps); +TEST_F(sparsehash_to_object, sparse_hash_set) { + auto filler = py_test::examples(); + auto a = google::sparse_hash_set(filler.begin(), filler.end()); + py_test::test_set_to_object_impl(a); } -TEST_F(sparsehash_to_object, set_to_object) { +TEST_F(sparsehash_to_object, dense_hash_set) { auto filler = py_test::examples(); auto a = google::dense_hash_set(filler.begin(), filler.end(), "the_empty_key"s); py_test::test_set_to_object_impl(a); diff --git a/tests/test_to_object.cc b/tests/test_to_object.cc index 2c1a514b..d7ea5683 100644 --- a/tests/test_to_object.cc +++ b/tests/test_to_object.cc @@ -24,10 +24,8 @@ TEST_F(to_object, map_to_object) { // NOTE: This test takes a long time to compile (about a .5s per entry in this // tuple). This is just enough coverage to test all three of our hash table types, // and a few important key/value types. - auto maps = std::make_tuple(std::unordered_map()); - - // Call test_map_to_object_impl on each entry in ``maps``. - std::apply([&](auto... map) { (py_test::test_map_to_object_impl(map), ...); }, maps); + auto map = std::unordered_map(); + py_test::test_map_to_object_impl(map); } TEST_F(to_object, vector_to_object) { diff --git a/tests/test_utils.h b/tests/test_utils.h index 3aee9abd..6e4d9395 100644 --- a/tests/test_utils.h +++ b/tests/test_utils.h @@ -192,7 +192,7 @@ inline py::owned_ref<> run_python( namespace py_test { template -inline std::array examples(); +std::array examples(); template<> inline std::array examples() { @@ -233,7 +233,7 @@ inline std::array, 3> examples() { } template -inline void test_map_to_object_impl(M m) { +void test_map_to_object_impl(M m) { // Fill the map with some example values. auto it = py::zip(examples(), @@ -281,7 +281,7 @@ inline void test_map_to_object_impl(M m) { } template -inline void test_sequence_to_object_impl(V v) { +void test_sequence_to_object_impl(V v) { auto check_python_list = [&](py::owned_ref ob) { ASSERT_TRUE(ob) << "to_object should not return null"; EXPECT_EQ(PyList_Check(ob.get()), 1) << "ob should be a list"; @@ -320,7 +320,7 @@ inline void test_sequence_to_object_impl(V v) { } template -inline void test_set_to_object_impl(V v) { +void test_set_to_object_impl(V v) { auto check_python_set = [&](py::owned_ref ob) { ASSERT_TRUE(ob) << "to_object should not return null"; EXPECT_EQ(PySet_Check(ob.get()), 1) << "ob should be a set"; From 9610f91aa7a4a2d7d1d94bb3d3d9a82432f63ab4 Mon Sep 17 00:00:00 2001 From: Gerry Manoim Date: Thu, 2 Jul 2020 18:24:37 -0400 Subject: [PATCH 7/8] clang fmt --- include/libpy/library_wrappers/sparsehash.h | 3 --- tests/library_wrappers/test_sparsehash.cc | 5 +++-- tests/test_utils.h | 2 +- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/include/libpy/library_wrappers/sparsehash.h b/include/libpy/library_wrappers/sparsehash.h index 14c5e458..99132ac2 100644 --- a/include/libpy/library_wrappers/sparsehash.h +++ b/include/libpy/library_wrappers/sparsehash.h @@ -7,7 +7,6 @@ #include #include - #include "libpy/to_object.h" namespace py { @@ -155,7 +154,6 @@ template struct to_object> : public set_to_object> {}; - template struct to_object> : public map_to_object> {}; @@ -172,6 +170,5 @@ template struct to_object> : public set_to_object> {}; - } // namespace dispatch } // namespace py diff --git a/tests/library_wrappers/test_sparsehash.cc b/tests/library_wrappers/test_sparsehash.cc index 11ae6770..51c395ef 100644 --- a/tests/library_wrappers/test_sparsehash.cc +++ b/tests/library_wrappers/test_sparsehash.cc @@ -16,7 +16,6 @@ using namespace py::cs::literals; class sparsehash_to_object : public with_python_interpreter {}; - TEST_F(sparsehash_to_object, sparse_hash_map) { // NOTE: This test takes a long time to compile (about a .5s per entry in this // tuple). This is just enough coverage to test all three of our hash table types, @@ -41,7 +40,9 @@ TEST_F(sparsehash_to_object, sparse_hash_set) { TEST_F(sparsehash_to_object, dense_hash_set) { auto filler = py_test::examples(); - auto a = google::dense_hash_set(filler.begin(), filler.end(), "the_empty_key"s); + auto a = google::dense_hash_set(filler.begin(), + filler.end(), + "the_empty_key"s); py_test::test_set_to_object_impl(a); } diff --git a/tests/test_utils.h b/tests/test_utils.h index 6e4d9395..0d8252bf 100644 --- a/tests/test_utils.h +++ b/tests/test_utils.h @@ -358,4 +358,4 @@ void test_set_to_object_impl(V v) { check_python_set(rvalueref_result); } -} +} // namespace py_test From fd739566b6a373c677ef7bd1a5b8fe22923488d3 Mon Sep 17 00:00:00 2001 From: Gerry Manoim Date: Thu, 2 Jul 2020 18:31:23 -0400 Subject: [PATCH 8/8] drop comments --- tests/library_wrappers/test_sparsehash.cc | 4 ---- tests/test_to_object.cc | 3 --- 2 files changed, 7 deletions(-) diff --git a/tests/library_wrappers/test_sparsehash.cc b/tests/library_wrappers/test_sparsehash.cc index 51c395ef..dcdd882e 100644 --- a/tests/library_wrappers/test_sparsehash.cc +++ b/tests/library_wrappers/test_sparsehash.cc @@ -17,11 +17,7 @@ using namespace py::cs::literals; class sparsehash_to_object : public with_python_interpreter {}; TEST_F(sparsehash_to_object, sparse_hash_map) { - // NOTE: This test takes a long time to compile (about a .5s per entry in this - // tuple). This is just enough coverage to test all three of our hash table types, - // and a few important key/value types. auto map = google::sparse_hash_map(); - py_test::test_map_to_object_impl(map); } diff --git a/tests/test_to_object.cc b/tests/test_to_object.cc index d7ea5683..388a7972 100644 --- a/tests/test_to_object.cc +++ b/tests/test_to_object.cc @@ -21,9 +21,6 @@ using namespace py::cs::literals; class to_object : public with_python_interpreter {}; TEST_F(to_object, map_to_object) { - // NOTE: This test takes a long time to compile (about a .5s per entry in this - // tuple). This is just enough coverage to test all three of our hash table types, - // and a few important key/value types. auto map = std::unordered_map(); py_test::test_map_to_object_impl(map); }