You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datasketches.apache.org by GitBox <gi...@apache.org> on 2021/12/01 00:05:24 UTC
[GitHub] [datasketches-cpp] AlexanderSaydakov opened a new pull request #252: Theta and Tuple sketch setops sync with Java
AlexanderSaydakov opened a new pull request #252:
URL: https://github.com/apache/datasketches-cpp/pull/252
ported tests from Java, adjusted logic
--
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
[GitHub] [datasketches-cpp] AlexanderSaydakov merged pull request #252: Theta and Tuple sketch setops sync with Java
Posted by GitBox <gi...@apache.org>.
AlexanderSaydakov merged pull request #252:
URL: https://github.com/apache/datasketches-cpp/pull/252
--
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
[GitHub] [datasketches-cpp] coveralls edited a comment on pull request #252: Theta and Tuple sketch setops sync with Java
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #252:
URL: https://github.com/apache/datasketches-cpp/pull/252#issuecomment-983195595
## Pull Request Test Coverage Report for [Build 1551528232](https://coveralls.io/builds/44833415)
* **4** of **4** **(100.0%)** changed or added relevant lines in **2** files are covered.
* No unchanged relevant lines lost coverage.
* Overall coverage remained the same at **92.656%**
---
| Totals | [](https://coveralls.io/builds/44833415) |
| :-- | --: |
| Change from base [Build 1423399130](https://coveralls.io/builds/44030637): | 0.0% |
| Covered Lines: | 2170 |
| Relevant Lines: | 2342 |
---
##### 💛 - [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
[GitHub] [datasketches-cpp] coveralls edited a comment on pull request #252: Theta and Tuple sketch setops sync with Java
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #252:
URL: https://github.com/apache/datasketches-cpp/pull/252#issuecomment-983195595
## Pull Request Test Coverage Report for [Build 1546587823](https://coveralls.io/builds/44798275)
* **4** of **4** **(100.0%)** changed or added relevant lines in **2** files are covered.
* No unchanged relevant lines lost coverage.
* Overall coverage remained the same at **92.656%**
---
| Totals | [](https://coveralls.io/builds/44798275) |
| :-- | --: |
| Change from base [Build 1423399130](https://coveralls.io/builds/44030637): | 0.0% |
| Covered Lines: | 2170 |
| Relevant Lines: | 2342 |
---
##### 💛 - [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
[GitHub] [datasketches-cpp] AlexanderSaydakov merged pull request #252: Theta and Tuple sketch setops sync with Java
Posted by GitBox <gi...@apache.org>.
AlexanderSaydakov merged pull request #252:
URL: https://github.com/apache/datasketches-cpp/pull/252
--
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
[GitHub] [datasketches-cpp] jmalkin commented on a change in pull request #252: Theta and Tuple sketch setops sync with Java
Posted by GitBox <gi...@apache.org>.
jmalkin commented on a change in pull request #252:
URL: https://github.com/apache/datasketches-cpp/pull/252#discussion_r762138902
##########
File path: theta/include/theta_set_difference_base_impl.hpp
##########
@@ -36,7 +36,7 @@ seed_hash_(compute_seed_hash(seed))
template<typename EN, typename EK, typename CS, typename A>
template<typename FwdSketch, typename Sketch>
CS theta_set_difference_base<EN, EK, CS, A>::compute(FwdSketch&& a, const Sketch& b, bool ordered) const {
- if (a.is_empty() || a.get_num_retained() == 0 || b.is_empty()) return CS(a, ordered);
+ if (a.is_empty() || (a.get_num_retained() > 0 && b.is_empty())) return CS(a, true);
Review comment:
CS(a, true) will return a sorted compact sketch regardless of the value of the `ordered` input parameter. Not really wrong with a CompactSketch, but depending on usage it may force an unnecessary sort.
--
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
[GitHub] [datasketches-cpp] coveralls commented on pull request #252: Theta and Tuple sketch setops sync with Java
Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #252:
URL: https://github.com/apache/datasketches-cpp/pull/252#issuecomment-983195595
## Pull Request Test Coverage Report for [Build 1523456119](https://coveralls.io/builds/44658155)
* **2** of **2** **(100.0%)** changed or added relevant lines in **2** files are covered.
* No unchanged relevant lines lost coverage.
* Overall coverage remained the same at **92.656%**
---
| Totals | [](https://coveralls.io/builds/44658155) |
| :-- | --: |
| Change from base [Build 1423399130](https://coveralls.io/builds/44030637): | 0.0% |
| Covered Lines: | 2170 |
| Relevant Lines: | 2342 |
---
##### 💛 - [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