Skip to content

GH-40348: [Python] Add Python wrapper for VariableShapeTensor - #40354

Open
rok wants to merge 4 commits into
apache:mainfrom
rok:38007_python
Open

GH-40348: [Python] Add Python wrapper for VariableShapeTensor#40354
rok wants to merge 4 commits into
apache:mainfrom
rok:38007_python

Conversation

@rok

@rokrok commented Mar 4, 2024•
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Member

Rationale for this change

Once C++ implementation of VariableShapeTensor is complete (#38007) we want a Python wrapper for VariableShapeTensor.

What changes are included in this PR?

This adds a Python wrapper for VariableShapeTensor.

Are these changes tested?

This adds appropriate Python tests.

Are there any user-facing changes?

Yes, a new extension and array type are exposed.

@rok
rok requested a review from westonpace as a code ownerMarch 4, 2024 16:43
@rokrok mentioned this pull request Mar 4, 2024
Merged
@github-actionsgithub-actionsBot added the awaiting committer review Awaiting committer review label Mar 4, 2024
AlenkaF
AlenkaF reviewed Mar 12, 2024
View reviewed changes

@AlenkaFAlenkaF left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small nits =) Thank you for working on this, takes lots of work!

Comment threadpython/pyarrow/array.pxi Outdated
Comment threadpython/pyarrow/array.pxi Outdated
@rok
rokforce-pushed the 38007_python branch 8 times, most recently from b6b63a6 to a372fe6CompareApril 6, 2024 10:24
@github-actionsgithub-actionsBot added awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review labels Apr 6, 2024
@AlenkaF

AlenkaF commented Apr 6, 2024

Copy link
Copy Markdown
Member

The failures are connected.

@github-actionsgithub-actionsBot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Apr 6, 2024
@rokrok mentioned this pull request May 8, 2024
Closed
@rok
rokforce-pushed the 38007_python branch 4 times, most recently from fab047e to f58c973CompareJune 7, 2024 00:28
@rok
rokforce-pushed the 38007_python branch 2 times, most recently from 5197532 to 55cc40bCompareAugust 26, 2024 17:55
@github-actionsgithub-actionsBot added the Status: stale-warning Issues and PRs flagged as stale which are due to be closed if no indication otherwise label Nov 18, 2025
@github-actionsgithub-actionsBot added the awaiting change review Awaiting change review label Feb 27, 2026
@rokrok mentioned this pull request Feb 27, 2026
Open
@apacheapache deleted a comment from github-actionsBotFeb 27, 2026
@rok
rokforce-pushed the 38007_python branch 3 times, most recently from 0df8139 to d5a548dCompareMarch 3, 2026 17:45
@rok

rok commented Mar 17, 2026

Copy link
Copy Markdown
MemberAuthor

@AlenkaF could you do a quick pass on this? I'm a little unsure about the *row_splits APIs, here's more about the idea.

@AlenkaF

AlenkaF commented Mar 18, 2026

Copy link
Copy Markdown
Member

@rok sure! Will have a look later today.

@AlenkaF

AlenkaF commented Mar 19, 2026

Copy link
Copy Markdown
Member

I did a very quick look at the code now. I would suggest splitting this PR into two as there is quite a lot of lines added and there might be even something that can be moved out.

Moving the *row_splits API related code into a separate issue and PR would be my suggestion. Otherwise, from first look, they sound useful. Might also be good to have a chat on the issue if it would be in fact useful for somebody?

@rok

rok commented Apr 14, 2026

Copy link
Copy Markdown
MemberAuthor

Moving the *row_splits API related code into a separate issue and PR would be my suggestion. Otherwise, from first look, they sound useful. Might also be good to have a chat on the issue if it would be in fact useful for somebody?

Agreed, I moved these into a separate PR and rebased here. I think this is ready for a more final review.

AlenkaF
AlenkaF reviewed Apr 15, 2026
View reviewed changes

@AlenkaFAlenkaF left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for adding this @rok! Huge amount of work was needed here (and in the C++ implementation).

I did a first check but still need to go through most of the Cython code and all the tests.
As for the C++ permutation change - maybe that also fits better into a separate issue&PR?

Comment threadpython/pyarrow/includes/libarrow.pxd Outdated
Comment threadpython/pyarrow/includes/libarrow.pxd Outdated
Comment threadpython/pyarrow/includes/libarrow.pxd Outdated
Comment threadpython/pyarrow/includes/libarrow.pxd Outdated
Comment threadpython/pyarrow/array.pxi Outdated
Comment threadpython/pyarrow/array.pxi Outdated
@github-actionsgithub-actionsBot added awaiting changes Awaiting changes and removed awaiting change review Awaiting change review labels Apr 15, 2026
@rok

rok commented Apr 15, 2026

Copy link
Copy Markdown
MemberAuthor

Thanks for the quick review @AlenkaF ! I addressed your comments, please do another pass whenever you can.

Regarding the C++ permutation change - I'll think about it a bit more and perhaps really spin it out. Again, I'm not entirely sure what to do yet, but it doesn't change the remainder of the review.

@github-actionsgithub-actionsBot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Apr 15, 2026
rok added 4 commits April 16, 2026 22:03
@rok
apacheGH-38007: [Python] Add VariableShapeTensor Python bindings
589f481
Add PyArrow bindings for the VariableShapeTensor extension type,
including VariableShapeTensorType, VariableShapeTensorArray, and
VariableShapeTensorScalar with support for converting to/from
NumPy tensors.
@rok
review feedback
4c85aa0
@rok
Revert the C++ change
bf4d995
@rok
fix permutation inference
0a57c1c
raulcd
raulcd reviewed Apr 27, 2026
View reviewed changes

@raulcdraulcd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The failures on CI seem related. @rok Is this ready for review?

@github-actionsgithub-actionsBot added awaiting changes Awaiting changes and removed awaiting change review Awaiting change review labels Apr 27, 2026
@rok

rok commented Apr 27, 2026

Copy link
Copy Markdown
MemberAuthor

@raulcd yeah, it's related. I wanted to reason through strides/permutation logic one more time. The rest of the PR is ready for review.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@raulcdraulcdraulcd left review comments
@AlenkaFAlenkaFAlenkaF left review comments
@westonpacewestonpaceAwaiting requested review from westonpace
@jorisvandenbosschejorisvandenbosscheAwaiting requested review from jorisvandenbossche

Assignees

No one assigned

Labels

awaiting changesAwaiting changesComponent: C++Component: DocumentationComponent: Python

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@rok@AlenkaF@raulcd