Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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-33984: [C++][Python] DLPack implementation for Arrow Arrays (producer) #38472
GH-33984: [C++][Python] DLPack implementation for Arrow Arrays (producer) #38472
Changes from 72 commits
f8f430f
cf143cd
d43367f
5778a33
1f0e100
e23249e
3d8c581
82a270f
00ac266
aeb20a6
fadf1f9
72ffcf3
c1ec84e
eabc58f
f834d27
3556b07
57ceee0
78363df
22739e8
1bcf161
c8d8799
2a5bf42
21b95d8
1762544
fedd464
01997a6
f8dbb0b
c6ee1bb
bcd05ea
6626a91
d169d4c
11d48af
5742e1d
024f535
e6d927c
da9baf2
5e2bb80
3af44e1
1f81277
cb0a942
46206ec
0619c35
55246ea
0fa84de
010f28e
53f2867
f9fbf2c
ab88549
c832edd
0089e23
804878f
fea6fe3
8071c9b
9f21208
8a10e68
672043b
ac85f4e
7e0d5f8
0fc962c
0d81ae0
d232de4
602e7b5
d9b3182
015ac3d
d2eb7c8
15ead8f
5e87138
2160ecd
8bb7173
9389941
94dec4b
811d2b5
49a978f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Do we plan to later support
fixed_shape_tensor
extension arrays?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.
See #38868 for that
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.
nitpick: I think it's best practice to use
std::make_unique
instead of wrappingnew
call. Any reason we can't usemake_unique
here?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.
Happy to change to
make_unique
. @pitrou any thoughts?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.
Given that we have the explicit
delete
as well, I personally like seeing thenew
+delete
combo explicitly, even thoughmake_unique
is exactly equivalent AFAIUThere 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.
Unless that
delete
call can then be replaced with a unique_ptrreset()
?