Add Doxygen Documentation for libcuspatial
#534
Conversation
Co-authored-by: Mark Harris <mharris@nvidia.com>
isVoid
added
doc
|
|
||
| /** | ||
| * @internal | ||
| * @brief Helper to convert a tuple of elements into a `vec_2d` |
There was a problem hiding this comment.
This question is out of scope for this PR, but why a tuple and not a pair? Issues with thrust::pair?
There was a problem hiding this comment.
why is a pair preferable to a tuple?
There was a problem hiding this comment.
Preferable is both subjective and circumstantial. As far as subjective arguments, on one hand we're making a 2d vector, which is exactly a pair of values, so it seems intuitive to use a pair. On the other hand I guess you could argue that you might want to generalize this to other dimensionality and returning a tuple across all dimensions would be more consistent. It's also convenient to use pair.first/second accessors rather than having to call a method.
As far as circumstantial arguments, the only real important thing that I can see is that in some situations you might care that pairs are standard layout while tuples are not. I don't think that really matters for us here.
I don't particularly think that we need to change this, just thought I'd check.
|
I've addressed my own comments on this PR and I approve. I think we can continue further discussions on the appropriate groups etc in the future, but those shouldn't block merging of this PR. |
|
rerun tests |
|
@gpucibot merge |
This PR adds doxygen documentation for libcuspatial.
In general, groupings to c++ APIs per our discussion on cuspatial method categories. There are 4 main categories:
I took liberty on sub-categories and is subject to discussion and change, as part of discussion in #525 .
Many docstring for APIs were missing and are added in this PR. Such as factory methods in
type_utils.hpp.Some docstrings broke when being rendered in pages, such as
point_in_polygon, these were also fixed.