You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datasketches.apache.org by jm...@apache.org on 2022/06/03 06:11:52 UTC

[datasketches-cpp] 01/01: fixes for type converting constructor, align documentation with kll

This is an automated email from the ASF dual-hosted git repository.

jmalkin pushed a commit to branch quantiles_patch
in repository https://gitbox.apache.org/repos/asf/datasketches-cpp.git

commit 65a3002ea7b3228dcf74abe5dc83e6633a2135e2
Author: Jon Malkin <jm...@users.noreply.github.com>
AuthorDate: Thu Jun 2 23:11:39 2022 -0700

    fixes for type converting constructor, align documentation with kll
---
 quantiles/include/quantiles_sketch.hpp      |  5 +++++
 quantiles/include/quantiles_sketch_impl.hpp |  9 ++++----
 quantiles/test/quantiles_sketch_test.cpp    | 32 +++++++++++++++++++++++++++++
 3 files changed, 41 insertions(+), 5 deletions(-)

diff --git a/quantiles/include/quantiles_sketch.hpp b/quantiles/include/quantiles_sketch.hpp
index acf430a..b09c552 100644
--- a/quantiles/include/quantiles_sketch.hpp
+++ b/quantiles/include/quantiles_sketch.hpp
@@ -162,6 +162,11 @@ public:
   quantiles_sketch& operator=(const quantiles_sketch& other);
   quantiles_sketch& operator=(quantiles_sketch&& other) noexcept;
 
+  /**
+   * @brief Type converting constructor
+   * @param other quantiles sketch of a different type
+   * @param allocator instance of an Allocator
+   */
   template<typename From, typename FC, typename FA>
   explicit quantiles_sketch(const quantiles_sketch<From, FC, FA>& other, const Allocator& allocator = Allocator());
 
diff --git a/quantiles/include/quantiles_sketch_impl.hpp b/quantiles/include/quantiles_sketch_impl.hpp
index e9fb4f7..f1df570 100644
--- a/quantiles/include/quantiles_sketch_impl.hpp
+++ b/quantiles/include/quantiles_sketch_impl.hpp
@@ -151,9 +151,8 @@ min_value_(nullptr),
 max_value_(nullptr),
 is_sorted_(false)
 {
-  static_assert(std::is_convertible<From, T>::value
-                || std::is_constructible<From, T>::value,
-                "Copy constructor across types requires std::is_convertible or std::is_constructible");
+  static_assert(std::is_constructible<T, From>::value,
+                "Type converting constructor requires new type to be constructible from existing type");
 
   base_buffer_.reserve(2 * std::min(quantiles_constants::MIN_K, k_));
 
@@ -174,7 +173,7 @@ is_sorted_(false)
     for (auto pair : other) {
       const uint64_t wt = pair.second;
       if (wt == 1) {
-        base_buffer_.push_back(pair.first);
+        base_buffer_.push_back(T(pair.first));
         // resize where needed as if adding points via update()
         if (base_buffer_.size() + 1 > base_buffer_.capacity()) {
           const size_t new_size = std::max(std::min(static_cast<size_t>(2 * k_), 2 * base_buffer_.size()), static_cast<size_t>(1));
@@ -183,7 +182,7 @@ is_sorted_(false)
       }
       else {
         const uint8_t idx = count_trailing_zeros_in_u64(pair.second) - 1;
-        levels_[idx].push_back(pair.first);
+        levels_[idx].push_back(T(pair.first));
       }
     }
 
diff --git a/quantiles/test/quantiles_sketch_test.cpp b/quantiles/test/quantiles_sketch_test.cpp
index 6e5de34..3effada 100644
--- a/quantiles/test/quantiles_sketch_test.cpp
+++ b/quantiles/test/quantiles_sketch_test.cpp
@@ -934,6 +934,38 @@ TEST_CASE("quantiles sketch", "[quantiles_sketch]") {
     }
   }
 
+  class A {
+    int val;
+  public:
+    A(int val): val(val) {}
+    int get_val() const { return val; }
+  };
+
+  struct less_A {
+    bool operator()(const A& a1, const A& a2) const { return a1.get_val() < a2.get_val(); }
+  };
+
+  class B {
+    int val;
+  public:
+    explicit B(const A& a): val(a.get_val()) {}
+    int get_val() const { return val; }
+  };
+
+  struct less_B {
+    bool operator()(const B& b1, const B& b2) const { return b1.get_val() < b2.get_val(); }
+  };
+
+  SECTION("type conversion: custom types") {
+    quantiles_sketch<A, less_A> sa;
+    sa.update(1);
+    sa.update(2);
+    sa.update(3);
+
+    quantiles_sketch<B, less_B> sb(sa);
+    REQUIRE(sb.get_n() == 3);
+  }
+
   // cleanup
   if (test_allocator_total_bytes != 0) {
     REQUIRE(test_allocator_total_bytes == 0);


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org