Skip to content

GH-40318: [C++][Docs] Add documentation of array factories#40373

Merged
pitrou merged 2 commits intoapache:mainfrom
vyasr:doc/column_factories
Mar 7, 2024
Merged

GH-40318: [C++][Docs] Add documentation of array factories#40373
pitrou merged 2 commits intoapache:mainfrom
vyasr:doc/column_factories

Conversation

@vyasr
Copy link
Copy Markdown
Contributor

@vyasr vyasr commented Mar 5, 2024

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-actions
Copy link
Copy Markdown

github-actions bot commented Mar 5, 2024

⚠️ GitHub issue #40318 has been automatically assigned in GitHub to PR creator.

@vyasr
Copy link
Copy Markdown
Contributor Author

vyasr commented Mar 5, 2024

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.

@vyasr
Copy link
Copy Markdown
Contributor Author

vyasr commented Mar 5, 2024

The CI test failures look odd and unrelated to the changeset.

@kou kou changed the title GH-40318:[C++] Add documentation of array factories GH-40318: [C++][Docs] Add documentation of array factories Mar 6, 2024
@kou
Copy link
Copy Markdown
Member

kou commented Mar 6, 2024

@github-actions crossbow submit preview-docs

@kou
Copy link
Copy Markdown
Member

kou commented Mar 6, 2024

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.

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.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 6, 2024

Revision: 098503b

Submitted crossbow builds: ursacomputing/crossbow @ actions-d7141ab764

Task Status
preview-docs GitHub Actions

@vyasr
Copy link
Copy Markdown
Contributor Author

vyasr commented Mar 6, 2024

Looks like the Cython 3.0.9 release yesterday broke the Python build. I can open a separate issue.

@vyasr
Copy link
Copy Markdown
Contributor Author

vyasr commented Mar 6, 2024

Following up on #40386 and #40387

@pitrou
Copy link
Copy Markdown
Member

pitrou commented Mar 7, 2024

@github-actions crossbow submit preview-docs

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 7, 2024

Revision: d04e32b

Submitted crossbow builds: ursacomputing/crossbow @ actions-f96c10210b

Task Status
preview-docs GitHub Actions

@kou
Copy link
Copy Markdown
Member

kou commented Mar 7, 2024

Copy link
Copy Markdown
Member

@pitrou pitrou left a 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!

@github-actions github-actions bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Mar 7, 2024
@pitrou pitrou merged commit 1c9a312 into apache:main Mar 7, 2024
@pitrou pitrou removed the awaiting committer review Awaiting committer review label Mar 7, 2024
@conbench-apache-arrow
Copy link
Copy Markdown

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.

@vyasr vyasr deleted the doc/column_factories branch March 8, 2024 04:28
rapids-bot bot pushed a commit to rapidsai/cudf that referenced this pull request Mar 8, 2024
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants