You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datasketches.apache.org by "AlexanderSaydakov (via GitHub)" <gi...@apache.org> on 2023/10/27 00:43:59 UTC
[PR] doxygen fixes [datasketches-cpp]
AlexanderSaydakov opened a new pull request, #400:
URL: https://github.com/apache/datasketches-cpp/pull/400
(no comment)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
Re: [PR] doxygen fixes [datasketches-cpp]
Posted by "jmalkin (via GitHub)" <gi...@apache.org>.
jmalkin commented on code in PR #400:
URL: https://github.com/apache/datasketches-cpp/pull/400#discussion_r1373972137
##########
tuple/include/tuple_a_not_b.hpp:
##########
@@ -37,11 +37,19 @@ class tuple_a_not_b {
using AllocEntry = typename std::allocator_traits<Allocator>::template rebind_alloc<Entry>;
using State = theta_set_difference_base<Entry, ExtractKey, CompactSketch, AllocEntry>;
+ /**
+ * Constructor
+ * @param seed for the hash function that was used to create the sketch
+ * @param allocator to use for allocating and deallocating memory
+ */
explicit tuple_a_not_b(uint64_t seed = DEFAULT_SEED, const Allocator& allocator = Allocator());
/**
* Computes the a-not-b set operation given two sketches.
- * @return the result of a-not-b
+ * @param a sketch A
Review Comment:
Same as above with A vs a but probably don't bother changing either place.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
Re: [PR] doxygen fixes [datasketches-cpp]
Posted by "AlexanderSaydakov (via GitHub)" <gi...@apache.org>.
AlexanderSaydakov commented on code in PR #400:
URL: https://github.com/apache/datasketches-cpp/pull/400#discussion_r1373979107
##########
tuple/include/array_tuple_sketch.hpp:
##########
@@ -116,6 +111,10 @@ template<typename Array, typename Allocator> class compact_array_tuple_sketch;
/**
* Update array tuple sketch.
+ * This is a wrapper around tuple sketch to match the functionality and serialization format of ArrayOfDoublesSketch in Java.
+ * For this the sketch must be configured with array<double> or std::vector<double>.
+ * This is a more generic implementation for any integral type (serialization assumes contiguous array size_of(T) * num_values).
Review Comment:
right, integral does not include floating point. arithmetic, perhaps
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
Re: [PR] doxygen fixes [datasketches-cpp]
Posted by "jmalkin (via GitHub)" <gi...@apache.org>.
jmalkin commented on code in PR #400:
URL: https://github.com/apache/datasketches-cpp/pull/400#discussion_r1373970932
##########
tuple/include/array_tuple_a_not_b.hpp:
##########
@@ -28,14 +28,27 @@
namespace datasketches {
+/// array tuple A-not-B
template<typename Array, typename Allocator = typename Array::allocator_type>
class array_tuple_a_not_b: tuple_a_not_b<Array, Allocator> {
public:
using Base = tuple_a_not_b<Array, Allocator>;
using CompactSketch = compact_array_tuple_sketch<Array, Allocator>;
+ /**
+ * Constructor
+ * @param seed for the hash function that was used to create the sketch
+ * @param allocator to use for allocating and deallocating memory
+ */
explicit array_tuple_a_not_b(uint64_t seed = DEFAULT_SEED, const Allocator& allocator = Allocator());
+ /**
+ * Computes the a-not-b set operation given two sketches.
+ * @param a sketch A
+ * @param b sketch B
+ * @param ordered optional flag to specify if ordered sketch should be produced
+ * @return the result of a-not-b as a compact sketch
Review Comment:
Nevermind. Variables are lowercase. Maybe call them a and b in the above @param lines?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
Re: [PR] doxygen fixes [datasketches-cpp]
Posted by "AlexanderSaydakov (via GitHub)" <gi...@apache.org>.
AlexanderSaydakov merged PR #400:
URL: https://github.com/apache/datasketches-cpp/pull/400
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
Re: [PR] doxygen fixes [datasketches-cpp]
Posted by "jmalkin (via GitHub)" <gi...@apache.org>.
jmalkin commented on code in PR #400:
URL: https://github.com/apache/datasketches-cpp/pull/400#discussion_r1373971671
##########
tuple/include/array_tuple_sketch.hpp:
##########
@@ -116,6 +111,10 @@ template<typename Array, typename Allocator> class compact_array_tuple_sketch;
/**
* Update array tuple sketch.
+ * This is a wrapper around tuple sketch to match the functionality and serialization format of ArrayOfDoublesSketch in Java.
+ * For this the sketch must be configured with array<double> or std::vector<double>.
+ * This is a more generic implementation for any integral type (serialization assumes contiguous array size_of(T) * num_values).
Review Comment:
Numeric type, not integral
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
Re: [PR] doxygen fixes [datasketches-cpp]
Posted by "coveralls (via GitHub)" <gi...@apache.org>.
coveralls commented on PR #400:
URL: https://github.com/apache/datasketches-cpp/pull/400#issuecomment-1782126414
## Pull Request Test Coverage Report for [Build 6661417137](https://coveralls.io/builds/63575694)
* **0** of **0** changed or added relevant lines in **0** files are covered.
* No unchanged relevant lines lost coverage.
* Overall coverage remained the same at **98.941%**
---
| Totals | [](https://coveralls.io/builds/63575694) |
| :-- | --: |
| Change from base [Build 6660871759](https://coveralls.io/builds/63574842): | 0.0% |
| Covered Lines: | 16070 |
| Relevant Lines: | 16242 |
---
##### 💛 - [Coveralls](https://coveralls.io)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
Re: [PR] doxygen fixes [datasketches-cpp]
Posted by "jmalkin (via GitHub)" <gi...@apache.org>.
jmalkin commented on code in PR #400:
URL: https://github.com/apache/datasketches-cpp/pull/400#discussion_r1373970567
##########
tuple/include/array_tuple_a_not_b.hpp:
##########
@@ -28,14 +28,27 @@
namespace datasketches {
+/// array tuple A-not-B
template<typename Array, typename Allocator = typename Array::allocator_type>
class array_tuple_a_not_b: tuple_a_not_b<Array, Allocator> {
public:
using Base = tuple_a_not_b<Array, Allocator>;
using CompactSketch = compact_array_tuple_sketch<Array, Allocator>;
+ /**
+ * Constructor
+ * @param seed for the hash function that was used to create the sketch
+ * @param allocator to use for allocating and deallocating memory
+ */
explicit array_tuple_a_not_b(uint64_t seed = DEFAULT_SEED, const Allocator& allocator = Allocator());
+ /**
+ * Computes the a-not-b set operation given two sketches.
+ * @param a sketch A
+ * @param b sketch B
+ * @param ordered optional flag to specify if ordered sketch should be produced
+ * @return the result of a-not-b as a compact sketch
Review Comment:
Maybe A-not-B is more consistent with the input sketch names?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org