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 2020/06/26 13:42:13 UTC
[GitHub] [incubator-datasketches-cpp] mdhimes opened a new pull request #161: Fix vector_of_kll_sketches check for number of elements
mdhimes opened a new pull request #161:
URL: https://github.com/apache/incubator-datasketches-cpp/pull/161
The current check throws an error for 2D arrays since it looks at the 0th axis for the number of elements, instead of the last axis (ndims-1). The array indexing is coded to assume the number of elements is in the last axis, so I updated the check to match this.
----------------------------------------------------------------
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.
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] [incubator-datasketches-cpp] mdhimes commented on pull request #161: Fix vector_of_kll_sketches check for number of elements
Posted by GitBox <gi...@apache.org>.
mdhimes commented on pull request #161:
URL: https://github.com/apache/incubator-datasketches-cpp/pull/161#issuecomment-652009653
Yeah, I see now that the unchecked approach is better. I had misinterpreted part of the pybind docs, and I thought the result of the indexing issue (seeing what looked like unallocated memory) was due to the object not being densely packed.
----------------------------------------------------------------
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.
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] [incubator-datasketches-cpp] jmalkin commented on pull request #161: Fix vector_of_kll_sketches check for number of elements
Posted by GitBox <gi...@apache.org>.
jmalkin commented on pull request #161:
URL: https://github.com/apache/incubator-datasketches-cpp/pull/161#issuecomment-650438808
Looks good. Should probably have a unit test that tests 1-d, 2-d, and some failure case. I'll try to add that if you don't get to it first.
----------------------------------------------------------------
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.
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] [incubator-datasketches-cpp] jmalkin merged pull request #161: Fix vector_of_kll_sketches check for number of elements
Posted by GitBox <gi...@apache.org>.
jmalkin merged pull request #161:
URL: https://github.com/apache/incubator-datasketches-cpp/pull/161
----------------------------------------------------------------
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.
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