-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
GH-40318: [C++][Docs] Add documentation of array factories #40373
Conversation
|
Question, are there bots set up to close the associated issue when this PR merges? Normally I would comment with one of Github's recognized keywords (Closes #X) but I'm guessing that's not necessary here. |
The CI test failures look odd and unrelated to the changeset. |
@github-actions crossbow submit preview-docs |
We'll do it by our merge script: https://github.com/apache/arrow/blob/main/dev/merge_arrow_pr.py So you don't need to do anything. |
Revision: 098503b Submitted crossbow builds: ursacomputing/crossbow @ actions-d7141ab764
|
Looks like the Cython 3.0.9 release yesterday broke the Python build. I can open a separate issue. |
@github-actions crossbow submit preview-docs |
Revision: d04e32b Submitted crossbow builds: ursacomputing/crossbow @ actions-f96c10210b
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good addition to the docs, thank you!
After merging your PR, Conbench analyzed the 5 benchmarking runs that have been run so far on merge-commit 1c9a312. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. |
It looks like soon after I started investigating scalar conversions for #14121 (but well before I made the PR) a major underlying hole was plugged in pyarrow via apache/arrow#36162. Most of #14121 was created to give us a way to handle scalars from pyarrow generically in libcudf. Now that pyarrow scalars can be easily tossed into arrays, we no longer really need separate scalar functions in libcudf; we can simply create an array from the scalar, put it into a table, and then call the table function. Additionally, arrow also has a function for creating an array from a scalar. This function is not new but [was previously undocumented](apache/arrow#40373). The builder code added to libcudf in #14121 can be removed and replaced with that factory. The scalar conversion is as simple as calling that arrow function and then using our preexisting `from_arrow` function on the resulting array. For now this PR is just a simplification of internals. Future PRs will remove the scalar API once we have a more standard path for the conversion of arrays via the C Data Interface. Authors: - Vyas Ramasubramani (https://github.com/vyasr) Approvers: - Yunsong Wang (https://github.com/PointKernel) - David Wendt (https://github.com/davidwendt) - Bradley Dice (https://github.com/bdice) URL: #15213
…che#40373) ### Rationale for this change These factory functions are generally useful and available, so documenting them helps external users find them without having to search the source code. ### What changes are included in this PR? This PR adds the array factories in arrow/array/util.h into a doxygen group for array factories and adds that group to the Sphinx C++ API documentation. ### Are these changes tested? I built the docs locally to verify. ### Are there any user-facing changes? Nothing to the API, only docs. * GitHub Issue: apache#40318 Authored-by: Vyas Ramasubramani <[email protected]> Signed-off-by: Antoine Pitrou <[email protected]>
Rationale for this change
These factory functions are generally useful and available, so documenting them helps external users find them without having to search the source code.
What changes are included in this PR?
This PR adds the array factories in arrow/array/util.h into a doxygen group for array factories and adds that group to the Sphinx C++ API documentation.
Are these changes tested?
I built the docs locally to verify.
Are there any user-facing changes?
Nothing to the API, only docs.