From 72ba84f34ca8e351efbfeff99f6a8b943f491068 Mon Sep 17 00:00:00 2001 From: Jon Malkin <786705+jmalkin@users.noreply.github.com> Date: Mon, 22 Jan 2024 11:42:36 -0800 Subject: [PATCH 01/12] Fix version for 5.0.0 release --- version.cfg.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.cfg.in b/version.cfg.in index 576606df..0062ac97 100644 --- a/version.cfg.in +++ b/version.cfg.in @@ -1 +1 @@ -5.0.@DT@.@HHMM@ +5.0.0 From 28a560240a72cf3451d9024c4ffb87c476997da7 Mon Sep 17 00:00:00 2001 From: Jon Malkin <786705+jmalkin@users.noreply.github.com> Date: Tue, 30 Jan 2024 23:30:28 -0800 Subject: [PATCH 02/12] Mega-commit for cherry-picking ease. Fix joint C++/python object handling, ensure __str__ takes no parameters, test to_string methods --- CMakeLists.txt | 2 +- include/kernel_function.hpp | 9 +++++---- include/tuple_policy.hpp | 20 +++++++++++--------- src/count_wrapper.cpp | 2 +- src/cpc_wrapper.cpp | 2 +- src/datasketches.cpp | 17 +++++++++++++++++ src/density_wrapper.cpp | 11 +++++++---- src/ebpps_wrapper.cpp | 2 +- src/fi_wrapper.cpp | 2 +- src/hll_wrapper.cpp | 2 +- src/kll_wrapper.cpp | 2 +- src/quantiles_wrapper.cpp | 2 +- src/req_wrapper.cpp | 2 +- src/theta_wrapper.cpp | 4 ++-- src/tuple_wrapper.cpp | 23 +++++++---------------- src/vector_of_kll.cpp | 6 +++--- src/vo_wrapper.cpp | 4 ++-- tests/count_min_test.py | 4 ++++ tests/cpc_test.py | 4 ++++ tests/density_test.py | 4 ++++ tests/ebpps_test.py | 4 ++++ tests/fi_test.py | 3 +++ tests/hll_test.py | 4 ++++ tests/kll_test.py | 3 +++ tests/quantiles_test.py | 3 +++ tests/req_test.py | 3 +++ tests/theta_test.py | 4 ++++ tests/tuple_test.py | 4 ++++ tests/vector_of_kll_test.py | 5 +++++ tests/vo_test.py | 9 +++++++++ 30 files changed, 117 insertions(+), 49 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index e0e46f44..350ebc05 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -74,7 +74,7 @@ execute_process( list(APPEND CMAKE_PREFIX_PATH "${NB_DIR}") find_package(nanobind CONFIG REQUIRED) -nanobind_add_module(python MODULE LTO STABLE_ABI NB_STATIC) +nanobind_add_module(python MODULE LTO NOMINSIZE STABLE_ABI NB_STATIC) set_target_properties(python PROPERTIES PREFIX "" diff --git a/include/kernel_function.hpp b/include/kernel_function.hpp index 0f19b0f4..93bc6d2a 100644 --- a/include/kernel_function.hpp +++ b/include/kernel_function.hpp @@ -20,7 +20,8 @@ #include #include #include -#include +#include +#include #include #include @@ -37,7 +38,7 @@ namespace datasketches { * which native Python kernels ultimately inherit. The actual * kernels implement KernelFunction, as shown in KernelFunction.py */ -struct kernel_function { +struct kernel_function : public nb::intrusive_base { virtual double operator()(nb::handle& a, nb::handle& b) const = 0; virtual ~kernel_function() = default; }; @@ -71,7 +72,7 @@ struct KernelFunction : public kernel_function { * never need to use this directly. */ struct kernel_function_holder { - explicit kernel_function_holder(std::shared_ptr kernel) : _kernel(kernel) {} + explicit kernel_function_holder(kernel_function* kernel) : _kernel(kernel) {} kernel_function_holder(const kernel_function_holder& other) : _kernel(other._kernel) {} kernel_function_holder(kernel_function_holder&& other) : _kernel(std::move(other._kernel)) {} kernel_function_holder& operator=(const kernel_function_holder& other) { _kernel = other._kernel; return *this; } @@ -99,7 +100,7 @@ struct kernel_function_holder { } private: - std::shared_ptr _kernel; + nb::ref _kernel; }; } diff --git a/include/tuple_policy.hpp b/include/tuple_policy.hpp index 01653244..bd70309f 100644 --- a/include/tuple_policy.hpp +++ b/include/tuple_policy.hpp @@ -20,6 +20,8 @@ #include #include #include +#include +#include #ifndef _TUPLE_POLICY_HPP_ #define _TUPLE_POLICY_HPP_ @@ -33,10 +35,10 @@ namespace datasketches { * which native Python policies ultimately inherit. The actual * policies implement TuplePolicy, as shown in TuplePolicy.py */ -struct tuple_policy { - virtual nb::object create_summary() = 0; - virtual nb::object update_summary(nb::object& summary, const nb::object& update) = 0; - virtual nb::object operator()(nb::object& summary, const nb::object& update) = 0; +struct tuple_policy : public nb::intrusive_base { + virtual nb::object create_summary() const = 0; + virtual nb::object update_summary(nb::object& summary, const nb::object& update) const = 0; + virtual nb::object operator()(nb::object& summary, const nb::object& update) const = 0; virtual ~tuple_policy() = default; }; @@ -53,7 +55,7 @@ struct TuplePolicy : public tuple_policy { * * @return nb::object representing a new summary */ - nb::object create_summary() override { + nb::object create_summary() const override { NB_OVERRIDE_PURE( create_summary, // Name of function in C++ (must match Python name) // Argument(s) -- if any @@ -67,7 +69,7 @@ struct TuplePolicy : public tuple_policy { * @param update The new value with which to update the summary * @return nb::object The updated summary */ - nb::object update_summary(nb::object& summary, const nb::object& update) override { + nb::object update_summary(nb::object& summary, const nb::object& update) const override { NB_OVERRIDE_PURE( update_summary, // Name of function in C++ (must match Python name) summary, update // Arguments @@ -81,7 +83,7 @@ struct TuplePolicy : public tuple_policy { * @param update An update to apply to the current summary * @return nb::object The potentially modified summary */ - nb::object operator()(nb::object& summary, const nb::object& update) override { + nb::object operator()(nb::object& summary, const nb::object& update) const override { NB_OVERRIDE_PURE_NAME( "__call__", // Name of function in python operator(), // Name of function in C++ @@ -96,7 +98,7 @@ struct TuplePolicy : public tuple_policy { * never need to use this directly. */ struct tuple_policy_holder { - explicit tuple_policy_holder(std::shared_ptr policy) : _policy(policy) {} + explicit tuple_policy_holder(tuple_policy* policy) : _policy(policy) {} tuple_policy_holder(const tuple_policy_holder& other) : _policy(other._policy) {} tuple_policy_holder(tuple_policy_holder&& other) : _policy(std::move(other._policy)) {} tuple_policy_holder& operator=(const tuple_policy_holder& other) { _policy = other._policy; return *this; } @@ -113,7 +115,7 @@ struct tuple_policy_holder { } private: - std::shared_ptr _policy; + nb::ref _policy; }; /* A degenerate policy used to enable Jaccard Similarity on tuple sketches, diff --git a/src/count_wrapper.cpp b/src/count_wrapper.cpp index fc3bb6e7..96774a98 100644 --- a/src/count_wrapper.cpp +++ b/src/count_wrapper.cpp @@ -50,7 +50,7 @@ void bind_count_min_sketch(nb::module_ &m, const char* name) { "with 95% confidence, frequency estimates satisfy the 'relative_error' guarantee. " "Returns the number of hash functions that are required in order to achieve the specified " "confidence of the sketch. confidence = 1 - delta, with delta denoting the sketch failure probability.") - .def("__str__", &count_min_sketch::to_string, + .def("__str__", [](const count_min_sketch& sk) { return sk.to_string(); }, "Produces a string summary of the sketch") .def("to_string", &count_min_sketch::to_string, "Produces a string summary of the sketch") diff --git a/src/cpc_wrapper.cpp b/src/cpc_wrapper.cpp index 4c64962c..06c428ac 100644 --- a/src/cpc_wrapper.cpp +++ b/src/cpc_wrapper.cpp @@ -39,7 +39,7 @@ void init_cpc(nb::module_ &m) { ":type seed: int, optional" ) .def("__copy__", [](const cpc_sketch& sk){ return cpc_sketch(sk); }) - .def("__str__", &cpc_sketch::to_string, + .def("__str__", [](const cpc_sketch& sk) { return sk.to_string(); }, "Produces a string summary of the sketch") .def("to_string", &cpc_sketch::to_string, "Produces a string summary of the sketch") diff --git a/src/datasketches.cpp b/src/datasketches.cpp index 7271e879..7eb70a55 100644 --- a/src/datasketches.cpp +++ b/src/datasketches.cpp @@ -18,6 +18,11 @@ */ #include +#include + +// needed for sketches such as Density and Tuple which rely +// on a joint C++/Python object +#include namespace nb = nanobind; @@ -41,6 +46,18 @@ void init_kolmogorov_smirnov(nb::module_& m); void init_serde(nb::module_& m); NB_MODULE(_datasketches, m) { + // needed in conjunction with the counter.inl include above + nb::intrusive_init( + [](PyObject *o) noexcept { + nb::gil_scoped_acquire guard; + Py_INCREF(o); + }, + [](PyObject *o) noexcept { + nb::gil_scoped_acquire guard; + Py_DECREF(o); + } + ); + init_hll(m); init_kll(m); init_fi(m); diff --git a/src/density_wrapper.cpp b/src/density_wrapper.cpp index f77cf22b..3221d17b 100644 --- a/src/density_wrapper.cpp +++ b/src/density_wrapper.cpp @@ -20,7 +20,8 @@ #include #include #include -#include +#include +#include #include #include @@ -38,7 +39,7 @@ void bind_density_sketch(nb::module_ &m, const char* name) { using namespace datasketches; nb::class_>(m, name) - .def("__init__", [](density_sketch* sk, uint16_t k, uint32_t dim, std::shared_ptr kernel) + .def("__init__", [](density_sketch* sk, uint16_t k, uint32_t dim, kernel_function* kernel) { K holder(kernel); new (sk) density_sketch(k, dim, holder); }, @@ -67,7 +68,7 @@ void bind_density_sketch(nb::module_ &m, const char* name) { "Returns True if the sketch is in estimation mode, otherwise False") .def("get_estimate", &density_sketch::get_estimate, nb::arg("point"), "Returns an approximate density at the given point") - .def("__str__", &density_sketch::to_string, nb::arg("print_levels")=false, nb::arg("print_items")=false, + .def("__str__", [](const density_sketch& sk) { return sk.to_string(); }, "Produces a string summary of the sketch") .def("to_string", &density_sketch::to_string, nb::arg("print_levels")=false, nb::arg("print_items")=false, "Produces a string summary of the sketch") @@ -87,7 +88,7 @@ void bind_density_sketch(nb::module_ &m, const char* name) { ) .def_static( "deserialize", - [](const nb::bytes& bytes, std::shared_ptr kernel) { + [](const nb::bytes& bytes, kernel_function* kernel) { K holder(kernel); return density_sketch::deserialize(bytes.c_str(), bytes.size(), holder); }, @@ -108,6 +109,8 @@ void init_density(nb::module_ &m) { // generic kernel function nb::class_(m, "KernelFunction", + nb::intrusive_ptr( + [](kernel_function *kf, PyObject *po) noexcept { kf->set_self_py(po); }), "A generic base class from which user-defined kernels must inherit.") .def(nb::init()) .def("__call__", &kernel_function::operator(), nb::arg("a"), nb::arg("b"), diff --git a/src/ebpps_wrapper.cpp b/src/ebpps_wrapper.cpp index ffd935aa..d2f24f27 100644 --- a/src/ebpps_wrapper.cpp +++ b/src/ebpps_wrapper.cpp @@ -39,7 +39,7 @@ void bind_ebpps_sketch(nb::module_ &m, const char* name) { ":param k: Maximum number of samples in the sketch\n:type k: int\n" ) .def("__copy__", [](const ebpps_sketch& sk){ return ebpps_sketch(sk); }) - .def("__str__", &ebpps_sketch::to_string, + .def("__str__", [](const ebpps_sketch& sk) { return sk.to_string(); }, "Produces a string summary of the sketch") .def("to_string", [](const ebpps_sketch& sk, bool print_items) { diff --git a/src/fi_wrapper.cpp b/src/fi_wrapper.cpp index 5cfb82c5..c14debaf 100644 --- a/src/fi_wrapper.cpp +++ b/src/fi_wrapper.cpp @@ -52,7 +52,7 @@ void bind_fi_sketch(nb::module_ &m, const char* name) { ":type lg_max_k: int\n" ) .def("__copy__", [](const frequent_items_sketch& sk){ return frequent_items_sketch(sk); }) - .def("__str__", &frequent_items_sketch::to_string, nb::arg("print_items")=false, + .def("__str__", [](const frequent_items_sketch& sk) { return sk.to_string(); }, "Produces a string summary of the sketch") .def("to_string", &frequent_items_sketch::to_string, nb::arg("print_items")=false, "Produces a string summary of the sketch") diff --git a/src/hll_wrapper.cpp b/src/hll_wrapper.cpp index 37c7d56d..51041ad3 100644 --- a/src/hll_wrapper.cpp +++ b/src/hll_wrapper.cpp @@ -44,7 +44,7 @@ void init_hll(nb::module_ &m) { "HLL_8) at the cost of much higher initial memory use. Default (and recommended) is False.\n" ":type start_full_size: bool" ) - .def("__str__", (std::string (hll_sketch::*)(bool,bool,bool,bool) const) &hll_sketch::to_string, + .def("__str__", [](const hll_sketch& sk) { return sk.to_string(); }, "Produces a string summary of the sketch") .def("to_string", (std::string (hll_sketch::*)(bool,bool,bool,bool) const) &hll_sketch::to_string, nb::arg("summary")=true, nb::arg("detail")=false, nb::arg("aux_detail")=false, nb::arg("all")=false, diff --git a/src/kll_wrapper.cpp b/src/kll_wrapper.cpp index f283ec7b..694f6cff 100644 --- a/src/kll_wrapper.cpp +++ b/src/kll_wrapper.cpp @@ -47,7 +47,7 @@ void bind_kll_sketch(nb::module_ &m, const char* name) { "Updates the sketch with the given value") .def("merge", (void (kll_sketch::*)(const kll_sketch&)) &kll_sketch::merge, nb::arg("sketch"), "Merges the provided sketch into this one") - .def("__str__", &kll_sketch::to_string, + .def("__str__", [](const kll_sketch& sk) { return sk.to_string(); }, "Produces a string summary of the sketch") .def("to_string", &kll_sketch::to_string, nb::arg("print_levels")=false, nb::arg("print_items")=false, "Produces a string summary of the sketch") diff --git a/src/quantiles_wrapper.cpp b/src/quantiles_wrapper.cpp index 0b466da0..fe830025 100644 --- a/src/quantiles_wrapper.cpp +++ b/src/quantiles_wrapper.cpp @@ -52,7 +52,7 @@ void bind_quantiles_sketch(nb::module_ &m, const char* name) { ) .def("merge", (void (quantiles_sketch::*)(const quantiles_sketch&)) &quantiles_sketch::merge, nb::arg("sketch"), "Merges the provided sketch into this one") - .def("__str__", &quantiles_sketch::to_string, + .def("__str__", [](const quantiles_sketch& sk) { return sk.to_string(); }, "Produces a string summary of the sketch") .def("to_string", &quantiles_sketch::to_string, nb::arg("print_levels")=false, nb::arg("print_items")=false, "Produces a string summary of the sketch") diff --git a/src/req_wrapper.cpp b/src/req_wrapper.cpp index b02ccf7d..4f862b1f 100644 --- a/src/req_wrapper.cpp +++ b/src/req_wrapper.cpp @@ -49,7 +49,7 @@ void bind_req_sketch(nb::module_ &m, const char* name) { "Updates the sketch with the given value") .def("merge", (void (req_sketch::*)(const req_sketch&)) &req_sketch::merge, nb::arg("sketch"), "Merges the provided sketch into this one") - .def("__str__", &req_sketch::to_string, + .def("__str__", [](const req_sketch& sk) { return sk.to_string(); }, "Produces a string summary of the sketch") .def("to_string", &req_sketch::to_string, nb::arg("print_levels")=false, nb::arg("print_items")=false, "Produces a string summary of the sketch") diff --git a/src/theta_wrapper.cpp b/src/theta_wrapper.cpp index e8329c63..1e4eee56 100644 --- a/src/theta_wrapper.cpp +++ b/src/theta_wrapper.cpp @@ -36,9 +36,9 @@ void init_theta(nb::module_ &m) { using namespace datasketches; nb::class_(m, "theta_sketch", "An abstract base class for theta sketches") - .def("__str__", &theta_sketch::to_string, nb::arg("print_items")=false, + .def("__str__", [](const theta_sketch& sk) { return sk.to_string(); }, "Produces a string summary of the sketch") - .def("to_string", &theta_sketch::to_string, nb::arg("print_items")=false, + .def("to_string", [](const theta_sketch& sk, bool print_items) { return sk.to_string(print_items); }, nb::arg("print_items")=false, "Produces a string summary of the sketch") .def("is_empty", static_cast(&theta_sketch::is_empty), "Returns True if the sketch is empty, otherwise False") diff --git a/src/tuple_wrapper.cpp b/src/tuple_wrapper.cpp index a2067f4a..6e906a7b 100644 --- a/src/tuple_wrapper.cpp +++ b/src/tuple_wrapper.cpp @@ -21,8 +21,8 @@ #include #include #include +#include #include -#include #include #include "py_serde.hpp" @@ -46,6 +46,8 @@ void init_tuple(nb::module_ &m) { // * update sketch policy uses create_summary and update_summary // * set operation policies all use __call__ nb::class_(m, "TuplePolicy", + nb::intrusive_ptr( + [](tuple_policy *tp, PyObject *po) noexcept { tp->set_self_py(po); }), "An abstract base class for Tuple Policy objects. All custom policies must extend this class.") .def(nb::init()) .def("create_summary", &tuple_policy::create_summary, @@ -66,17 +68,6 @@ void init_tuple(nb::module_ &m) { ) ; - // potentially useful for debugging but not needed as a permanent - // object type in the library - /* - nb::class_(m, "TuplePolicyHolder") - .def(nb::init>(), nb::arg("policy")) - .def("create", &tuple_policy_holder::create, "Creates a new Summary object") - .def("update", &tuple_policy_holder::update, nb::arg("summary"), nb::arg("update"), - "Updates the provided summary using the data in update") - ; - */ - using py_tuple_sketch = tuple_sketch; using py_update_tuple = update_tuple_sketch; using py_compact_tuple = compact_tuple_sketch; @@ -86,7 +77,7 @@ void init_tuple(nb::module_ &m) { using py_tuple_jaccard_similarity = jaccard_similarity_base, tuple_intersection, pair_extract_key>; nb::class_(m, "tuple_sketch", "An abstract base class for tuple sketches.") - .def("__str__", &py_tuple_sketch::to_string, + .def("__str__", [](const py_tuple_sketch& sk) { return sk.to_string(); }, "Produces a string summary of the sketch") .def("to_string", &py_tuple_sketch::to_string, nb::arg("print_items")=false, "Produces a string summary of the sketch") @@ -153,7 +144,7 @@ void init_tuple(nb::module_ &m) { nb::class_(m, "update_tuple_sketch") .def("__init__", - [](py_update_tuple* sk, std::shared_ptr policy, uint8_t lg_k, double p, uint64_t seed) { + [](py_update_tuple* sk, tuple_policy* policy, uint8_t lg_k, double p, uint64_t seed) { tuple_policy_holder holder(policy); new (sk) py_update_tuple(py_update_tuple::builder(holder).set_lg_k(lg_k).set_p(p).set_seed(seed).build()); }, @@ -182,7 +173,7 @@ void init_tuple(nb::module_ &m) { nb::class_(m, "tuple_union") .def("__init__", - [](py_tuple_union* u, std::shared_ptr policy, uint8_t lg_k, double p, uint64_t seed) { + [](py_tuple_union* u, tuple_policy* policy, uint8_t lg_k, double p, uint64_t seed) { tuple_policy_holder holder(policy); new (u) py_tuple_union(py_tuple_union::builder(holder).set_lg_k(lg_k).set_p(p).set_seed(seed).build()); }, @@ -203,7 +194,7 @@ void init_tuple(nb::module_ &m) { nb::class_(m, "tuple_intersection") .def("__init__", - [](py_tuple_intersection* sk, std::shared_ptr policy, uint64_t seed) { + [](py_tuple_intersection* sk, tuple_policy* policy, uint64_t seed) { tuple_policy_holder holder(policy); new (sk) py_tuple_intersection(seed, holder); }, diff --git a/src/vector_of_kll.cpp b/src/vector_of_kll.cpp index f0e5f306..3bbe17b6 100644 --- a/src/vector_of_kll.cpp +++ b/src/vector_of_kll.cpp @@ -539,11 +539,11 @@ void bind_vector_of_kll_sketches(nb::module_ &m, const char* name) { .def("update", &vector_of_kll_sketches::update, nb::arg("items"), nb::arg("order") = "C", "Updates the sketch(es) with value(s). Must be a 1D array of size equal to the number of sketches. Can also be 2D array of shape (n_updates, n_sketches). If a sketch does not have a value to update, use np.nan. " " Order 'F' specifies a column-major (Fortran style) matrix; any other value assumes row-major (C style) matrix.") - .def("__str__", &vector_of_kll_sketches::to_string, nb::arg("print_levels")=false, nb::arg("print_items")=false, - "Produces a string summary of all sketches. Users should split the returned string by '\n\n'") + .def("__str__", [](const vector_of_kll_sketches& sk) { return sk.to_string(); }, + "Produces a string summary of all sketches. Users should split the returned string by '\\n\\n'") .def("to_string", &vector_of_kll_sketches::to_string, nb::arg("print_levels")=false, nb::arg("print_items")=false, - "Produces a string summary of all sketches. Users should split the returned string by '\n\n'") + "Produces a string summary of all sketches. Users should split the returned string by '\\n\\n'") .def("is_empty", &vector_of_kll_sketches::is_empty, "Returns whether the sketch(es) is(are) empty of not") .def("get_n", &vector_of_kll_sketches::get_n, diff --git a/src/vo_wrapper.cpp b/src/vo_wrapper.cpp index 5a2a9b3f..ef0eb671 100644 --- a/src/vo_wrapper.cpp +++ b/src/vo_wrapper.cpp @@ -40,7 +40,7 @@ void bind_vo_sketch(nb::module_ &m, const char* name) { ":param k: Maximum number of samples in the sketch\n:type k: int\n" ) .def("__copy__", [](const var_opt_sketch& sk){ return var_opt_sketch(sk); }) - .def("__str__", &var_opt_sketch::to_string, + .def("__str__", [](const var_opt_sketch& sk) { return sk.to_string(); }, "Produces a string summary of the sketch") .def("to_string", [](const var_opt_sketch& sk, bool print_items) { @@ -105,7 +105,7 @@ void bind_vo_union(nb::module_ &m, const char* name) { nb::class_>(m, name) .def(nb::init(), nb::arg("max_k")) - .def("__str__", &var_opt_union::to_string, + .def("__str__", [](const var_opt_union& sk) { return sk.to_string(); }, "Produces a string summary of the sketch") .def("to_string", &var_opt_union::to_string, "Produces a string summary of the sketch") diff --git a/tests/count_min_test.py b/tests/count_min_test.py index 09f2e12e..42b5d9bc 100644 --- a/tests/count_min_test.py +++ b/tests/count_min_test.py @@ -82,5 +82,9 @@ def test_count_min_example(self): for i in range(0, 2 * n): self.assertEqual(cm.get_estimate(i), new_cm.get_estimate(i)) + # finally just check that printing works as expected + self.assertGreater(len(cm.to_string()), 0) + self.assertEqual(len(cm.__str__()), len(cm.to_string())) + if __name__ == '__main__': unittest.main() diff --git a/tests/cpc_test.py b/tests/cpc_test.py index bf25be23..fb1b40ee 100644 --- a/tests/cpc_test.py +++ b/tests/cpc_test.py @@ -60,6 +60,10 @@ def test_cpc_example(self): new_cpc = cpc_sketch.deserialize(sk_bytes) self.assertFalse(new_cpc.is_empty()) + # finally just check that printing works as expected + self.assertGreater(len(cpc.to_string()), 0) + self.assertEqual(len(cpc.__str__()), len(cpc.to_string())) + def test_cpc_get_lg_k(self): lgk = 10 cpc = cpc_sketch(lgk) diff --git a/tests/density_test.py b/tests/density_test.py index 96b3ac64..4dd66f45 100644 --- a/tests/density_test.py +++ b/tests/density_test.py @@ -61,6 +61,10 @@ def test_density_sketch(self): sketch2 = density_sketch.deserialize(sk_bytes, GaussianKernel()) self.assertEqual(sketch.get_estimate([1.5, 2.5, 3.5]), sketch2.get_estimate([1.5, 2.5, 3.5])) + # check that printing works as expected + self.assertGreater(len(sketch.to_string(True, True)), 0) + self.assertEqual(len(sketch.__str__()), len(sketch.to_string())) + def test_density_merge(self): sketch1 = density_sketch(10, 2, GaussianKernel()) sketch1.update([0, 0]) diff --git a/tests/ebpps_test.py b/tests/ebpps_test.py index 9c37a54d..76a3d367 100644 --- a/tests/ebpps_test.py +++ b/tests/ebpps_test.py @@ -109,5 +109,9 @@ def test_ebpps_example(self): self.assertEqual(sk.c, rebuilt.c) self.assertEqual(sk.n, rebuilt.n) + # check that printing works as expected + self.assertGreater(len(sk.to_string(True)), 0) + self.assertEqual(len(sk.__str__()), len(sk.to_string())) + if __name__ == '__main__': unittest.main() diff --git a/tests/fi_test.py b/tests/fi_test.py index ae8d58e3..6d56aff6 100644 --- a/tests/fi_test.py +++ b/tests/fi_test.py @@ -132,6 +132,9 @@ def test_fi_items_example(self): self.assertGreater(new_fi.num_active_items, 0) self.assertEqual(5 * wt, new_fi.total_weight) + # check that printing works as expected + self.assertGreater(len(fi.to_string(True)), 0) + self.assertEqual(len(fi.__str__()), len(fi.to_string())) def test_fi_sketch(self): # only testing a few things not used in the above example diff --git a/tests/hll_test.py b/tests/hll_test.py index 32d762ee..fba7727b 100644 --- a/tests/hll_test.py +++ b/tests/hll_test.py @@ -67,6 +67,10 @@ def test_hll_example(self): new_hll.reset() self.assertTrue(new_hll.is_empty()) + # check that printing works as expected + self.assertGreater(len(hll.to_string(True, True, True, True)), 0) + self.assertEqual(len(hll.__str__()), len(hll.to_string())) + def test_hll_sketch(self): lgk = 8 n = 117 diff --git a/tests/kll_test.py b/tests/kll_test.py index bfad88d8..a03bb227 100644 --- a/tests/kll_test.py +++ b/tests/kll_test.py @@ -167,6 +167,9 @@ def test_kll_items_sketch(self): # reflect the same underlying distribtion self.assertFalse(ks_test(kll, new_kll, 0.001)) + # check that printing works as expected + self.assertGreater(len(kll.to_string(True, True)), 0) + self.assertEqual(len(kll.__str__()), len(kll.to_string())) if __name__ == '__main__': unittest.main() diff --git a/tests/quantiles_test.py b/tests/quantiles_test.py index 8096b60b..988db2f9 100644 --- a/tests/quantiles_test.py +++ b/tests/quantiles_test.py @@ -167,6 +167,9 @@ def test_quantiles_items_sketch(self): # reflect the same underlying distribtion self.assertFalse(ks_test(quantiles, new_quantiles, 0.001)) + # check that printing works as expected + self.assertGreater(len(quantiles.to_string(True, True)), 0) + self.assertEqual(len(quantiles.__str__()), len(quantiles.to_string())) if __name__ == '__main__': unittest.main() diff --git a/tests/req_test.py b/tests/req_test.py index 9aa34206..775dfd44 100644 --- a/tests/req_test.py +++ b/tests/req_test.py @@ -162,6 +162,9 @@ def test_req_items_sketch(self): self.assertEqual(req.get_quantile(0.7), new_req.get_quantile(0.7)) self.assertEqual(req.get_rank(str(n/4)), new_req.get_rank(str(n/4))) + # check that printing works as expected + self.assertGreater(len(req.to_string(True, True)), 0) + self.assertEqual(len(req.__str__()), len(req.to_string())) if __name__ == '__main__': unittest.main() diff --git a/tests/theta_test.py b/tests/theta_test.py index a30a74fc..8cdb2a79 100644 --- a/tests/theta_test.py +++ b/tests/theta_test.py @@ -48,6 +48,10 @@ def test_theta_basic_example(self): self.assertFalse(sk.is_empty()) self.assertEqual(sk.get_estimate(), new_sk.get_estimate()) + # check that printing works as expected + self.assertGreater(len(sk.to_string(True)), 0) + self.assertEqual(len(sk.__str__()), len(sk.to_string())) + count = 0 for hash in new_sk: self.assertLess(hash, new_sk.theta64) diff --git a/tests/tuple_test.py b/tests/tuple_test.py index 971ef8cb..fa09b0b3 100644 --- a/tests/tuple_test.py +++ b/tests/tuple_test.py @@ -75,6 +75,10 @@ def test_tuple_basic_example(self): cumSum += pair[1] self.assertEqual(cumSum, 5 * cts.num_retained) + # check that printing works as expected + self.assertGreater(len(sk.to_string(True)), 0) + self.assertEqual(len(sk.__str__()), len(sk.to_string())) + num = sk.num_retained sk.trim() self.assertLessEqual(sk.num_retained, num) diff --git a/tests/vector_of_kll_test.py b/tests/vector_of_kll_test.py index bd6961a4..706f3f9d 100644 --- a/tests/vector_of_kll_test.py +++ b/tests/vector_of_kll_test.py @@ -127,6 +127,11 @@ def test_kll_2Dupdates(self): np.testing.assert_allclose(kll.get_min_values(), smin) np.testing.assert_allclose(kll.get_max_values(), smax) + # check that printing works as expected + self.assertGreater(len(kll.to_string(True, True)), 0) + self.assertEqual(len(kll.__str__()), len(kll.to_string())) + + def test_kll_3Dupdates(self): # now test 3D update, which should fail k = 200 diff --git a/tests/vo_test.py b/tests/vo_test.py index 00f126ad..def29eb4 100644 --- a/tests/vo_test.py +++ b/tests/vo_test.py @@ -128,5 +128,14 @@ def geq_zero(x): self.assertEqual(summary1['estimate'], summary2['estimate']) self.assertEqual(summary1['total_sketch_weight'], summary2['total_sketch_weight']) + # check that printing works as expected + self.assertGreater(len(result.to_string(True)), 0) + self.assertEqual(len(result.__str__()), len(result.to_string())) + + self.assertGreater(len(union.to_string()), 0) + self.assertEqual(len(union.__str__()), len(union.to_string())) + + + if __name__ == '__main__': unittest.main() From 6b1c38deac56c8fa87f2d24174f978a78fb64734 Mon Sep 17 00:00:00 2001 From: Jon Malkin <786705+jmalkin@users.noreply.github.com> Date: Wed, 7 Feb 2024 17:47:34 -0800 Subject: [PATCH 03/12] Fix to generate proper Apple Silicon wheels, bump versions to mitigate GH warnings --- .github/workflows/build_wheels.yml | 10 +++++++--- pyproject.toml | 3 --- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/.github/workflows/build_wheels.yml b/.github/workflows/build_wheels.yml index a668a676..54d139f8 100644 --- a/.github/workflows/build_wheels.yml +++ b/.github/workflows/build_wheels.yml @@ -69,10 +69,10 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Set up Python 3.x - uses: actions/setup-python@v4 + uses: actions/setup-python@v5 with: python-version: '3.x' @@ -88,8 +88,12 @@ jobs: - name: Build wheels run: python -m cibuildwheel --output-dir dist env: + CIBW_ENVIRONMENT_MACOS: CMAKE_OSX_ARCHITECTURES=${{ matrix.config.cibw-arch == 'macosx_x86_64' && 'x86_64' || matrix.config.cibw-arch == 'macosx_arm64' && 'arm64' || '' }} CIBW_BUILD: "*-${{ matrix.config.cibw-arch }}" + MACOSX_DEPLOYMENT_TARGET: "10.14" # min supporting c++17 - - uses: actions/upload-artifact@v3 + - uses: actions/upload-artifact@v4 with: + name: pypi_wheels path: ./dist/*.whl + compression-level: 0 # contents are already compressed diff --git a/pyproject.toml b/pyproject.toml index ff2becd5..5c584ec2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -36,6 +36,3 @@ before-build = "yum remove -y cmake" [tool.cibuildwheel.macos] archs = ["x86_64", "arm64"] - -# Minimum version for proper C++17 support on MacOS -environment = { MACOSX_DEPLOYMENT_TARGET = "10.14" } From 684f62e0ec9c3a7cb3c6d73102c8725a1c86f186 Mon Sep 17 00:00:00 2001 From: Jon Malkin <786705+jmalkin@users.noreply.github.com> Date: Wed, 7 Feb 2024 17:55:25 -0800 Subject: [PATCH 04/12] version bump dependencies for other workflows --- .github/workflows/ci.yml | 4 ++-- .github/workflows/sphinx.yml | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index adddde12..ae2699c4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -35,9 +35,9 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Set up Python 3.x - uses: actions/setup-python@v4 + uses: actions/setup-python@v5 # 3.x uses the latest minor version of python3. This may break briefly when there is a new version # but dependent libraries (e.g. numpy) have not yet released a compatible update. # May need to enable version pinning (e.g. 3.10) temporarily at times. diff --git a/.github/workflows/sphinx.yml b/.github/workflows/sphinx.yml index 1b30f344..12e864f6 100644 --- a/.github/workflows/sphinx.yml +++ b/.github/workflows/sphinx.yml @@ -11,9 +11,9 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Install Python - uses: actions/setup-python@v4 + uses: actions/setup-python@v5 with: python-version: '3.x' - name: Install Datasketches and Sphinx From 5c47bce5f45ec9bdd36c7b96f87c70e5d74a68df Mon Sep 17 00:00:00 2001 From: Jon Malkin <786705+jmalkin@users.noreply.github.com> Date: Wed, 7 Feb 2024 17:58:16 -0800 Subject: [PATCH 05/12] don't try to name aritfact --- .github/workflows/build_wheels.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/build_wheels.yml b/.github/workflows/build_wheels.yml index 54d139f8..7a6704c3 100644 --- a/.github/workflows/build_wheels.yml +++ b/.github/workflows/build_wheels.yml @@ -94,6 +94,5 @@ jobs: - uses: actions/upload-artifact@v4 with: - name: pypi_wheels path: ./dist/*.whl compression-level: 0 # contents are already compressed From df73ee7bb545d5aac9ba38063557294315265ce5 Mon Sep 17 00:00:00 2001 From: Jon Malkin <786705+jmalkin@users.noreply.github.com> Date: Wed, 7 Feb 2024 18:02:06 -0800 Subject: [PATCH 06/12] bump versions for source artifact --- .github/workflows/build_wheels.yml | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/.github/workflows/build_wheels.yml b/.github/workflows/build_wheels.yml index 7a6704c3..c41dbc25 100644 --- a/.github/workflows/build_wheels.yml +++ b/.github/workflows/build_wheels.yml @@ -12,9 +12,9 @@ jobs: name: Source distribution runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - - uses: actions/setup-python@v4 + - uses: actions/setup-python@v5 name: Install Python with: python-version: '3.x' @@ -25,9 +25,11 @@ jobs: - name: Build sdist run: python -m build --sdist --outdir dist - - uses: actions/upload-artifact@v3 + - uses: actions/upload-artifact@v4 with: path: dist/*.tar.gz + compression-level: 0 # contents are already compressed + build_wheels: name: ${{ matrix.config.name }} From 074643a1ccdcd325a27a33a5161937ac27916980 Mon Sep 17 00:00:00 2001 From: Jon Malkin <786705+jmalkin@users.noreply.github.com> Date: Wed, 7 Feb 2024 19:04:59 -0800 Subject: [PATCH 07/12] unique artifact names and merge into one at the end --- .github/workflows/build_wheels.yml | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build_wheels.yml b/.github/workflows/build_wheels.yml index c41dbc25..2350e943 100644 --- a/.github/workflows/build_wheels.yml +++ b/.github/workflows/build_wheels.yml @@ -6,6 +6,7 @@ on: env: BUILD_TYPE: Release MIN_CIBUILDWHEEL_VERSION: 2.16.2 + PYTHON_VERSION: 3.11 jobs: build_sdist: @@ -17,7 +18,7 @@ jobs: - uses: actions/setup-python@v5 name: Install Python with: - python-version: '3.x' + python-version: ${{ env.PYTHON_VERSION }} - name: Install build package run: python -m pip install build --user @@ -27,6 +28,7 @@ jobs: - uses: actions/upload-artifact@v4 with: + name: sdist path: dist/*.tar.gz compression-level: 0 # contents are already compressed @@ -96,5 +98,19 @@ jobs: - uses: actions/upload-artifact@v4 with: + name: wheel-${{ matrix.config.os }}-${{ matrix.config.cibw-arch }} path: ./dist/*.whl compression-level: 0 # contents are already compressed + + + aggregate_artifact: + name: Aggregate artifact + runs-on: ubuntu-latest + needs: [build_sdist, build_wheels] + steps: + - name: Merge Artifacts + uses: actions/upload-artifact/merge@v4 + with: + name: python_wheels + compression-level: 0 + delete-merged: true From 5592101b9d89225b51f419c3ebfd1c4d6bad2106 Mon Sep 17 00:00:00 2001 From: Jon Malkin <786705+jmalkin@users.noreply.github.com> Date: Wed, 7 Feb 2024 19:06:07 -0800 Subject: [PATCH 08/12] use PYTHON_VERSION variable consistently --- .github/workflows/build_wheels.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build_wheels.yml b/.github/workflows/build_wheels.yml index 2350e943..96e84145 100644 --- a/.github/workflows/build_wheels.yml +++ b/.github/workflows/build_wheels.yml @@ -75,10 +75,10 @@ jobs: - name: Checkout uses: actions/checkout@v4 - - name: Set up Python 3.x + - name: Set up Python ${{ env.PYTHON_VERSION }} uses: actions/setup-python@v5 with: - python-version: '3.x' + python-version: ${{ env.PYTHON_VERSION }} - name: Set up QEMU for linux/arm64 builds if: runner.os == 'Linux' && matrix.config.use-qemu == true From c4ba296de73fdadc6e94572289d031633c259554 Mon Sep 17 00:00:00 2001 From: Jon Malkin <786705+jmalkin@users.noreply.github.com> Date: Wed, 7 Feb 2024 19:43:33 -0800 Subject: [PATCH 09/12] Bump qemu version, tidy up naming --- .github/workflows/build_wheels.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/build_wheels.yml b/.github/workflows/build_wheels.yml index 96e84145..c7453e27 100644 --- a/.github/workflows/build_wheels.yml +++ b/.github/workflows/build_wheels.yml @@ -6,7 +6,7 @@ on: env: BUILD_TYPE: Release MIN_CIBUILDWHEEL_VERSION: 2.16.2 - PYTHON_VERSION: 3.11 + PYTHON_VERSION: 3.x jobs: build_sdist: @@ -75,14 +75,14 @@ jobs: - name: Checkout uses: actions/checkout@v4 - - name: Set up Python ${{ env.PYTHON_VERSION }} + - name: Set up Python uses: actions/setup-python@v5 with: python-version: ${{ env.PYTHON_VERSION }} - name: Set up QEMU for linux/arm64 builds if: runner.os == 'Linux' && matrix.config.use-qemu == true - uses: docker/setup-qemu-action@v2 + uses: docker/setup-qemu-action@v3 with: platforms: arm64 @@ -98,13 +98,13 @@ jobs: - uses: actions/upload-artifact@v4 with: - name: wheel-${{ matrix.config.os }}-${{ matrix.config.cibw-arch }} + name: wheels-${{ matrix.config.os }}-${{ matrix.config.cibw-arch }} path: ./dist/*.whl compression-level: 0 # contents are already compressed - aggregate_artifact: - name: Aggregate artifact + aggregate_artifacts: + name: Aggregate artifacts runs-on: ubuntu-latest needs: [build_sdist, build_wheels] steps: From 3e71a481f31917c0292b70aa9b5a092a9e225718 Mon Sep 17 00:00:00 2001 From: Jon Malkin <786705+jmalkin@users.noreply.github.com> Date: Wed, 7 Feb 2024 20:14:04 -0800 Subject: [PATCH 10/12] v5.0.1 --- version.cfg.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.cfg.in b/version.cfg.in index 0062ac97..6b244dcd 100644 --- a/version.cfg.in +++ b/version.cfg.in @@ -1 +1 @@ -5.0.0 +5.0.1 From 9e8ac10a03d7865b8e378fc446411111a7c2dcdd Mon Sep 17 00:00:00 2001 From: Jon Date: Sat, 30 Mar 2024 00:43:18 -0700 Subject: [PATCH 11/12] apply numpy fix to 5.0.x branch --- src/density_wrapper.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/density_wrapper.cpp b/src/density_wrapper.cpp index 3221d17b..2bc422fe 100644 --- a/src/density_wrapper.cpp +++ b/src/density_wrapper.cpp @@ -98,7 +98,7 @@ void bind_density_sketch(nb::module_ &m, const char* name) { } int prepare_numpy() { - import_array(); + import_array1(0); return 0; } From 11915bb301c5cace64eebe220c4a984bf9e2ad14 Mon Sep 17 00:00:00 2001 From: Jon Date: Mon, 1 Apr 2024 18:57:21 -0700 Subject: [PATCH 12/12] bump patch version --- version.cfg.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.cfg.in b/version.cfg.in index 6b244dcd..a1ef0cae 100644 --- a/version.cfg.in +++ b/version.cfg.in @@ -1 +1 @@ -5.0.1 +5.0.2