Skip to content
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

ARROW-15277: [C++][Python] Use ChunkedArray::Make for chunked_array #13950

Merged
merged 9 commits into from
Aug 25, 2022
Merged

ARROW-15277: [C++][Python] Use ChunkedArray::Make for chunked_array #13950

merged 9 commits into from
Aug 25, 2022

Conversation

milesgranger
Copy link
Contributor

Supersedes and will close #12096

ARROW-15277

@github-actions
Copy link

@github-actions
Copy link

⚠️ Ticket has not been started in JIRA, please click 'Start Progress'.

@milesgranger
Copy link
Contributor Author

@pitrou I believe this is ready for a review.

@@ -143,14 +140,15 @@ def test_chunked_array_to_numpy():


def test_chunked_array_mismatch_types():
with pytest.raises(TypeError):
msg = "chunks must all be same type"
with pytest.raises(pa.ArrowInvalid, match=msg):
Copy link
Member

Choose a reason for hiding this comment

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

Hmm... interesting. I would be in favor of changing the C++ side to return TypeError instead. Would you like to do that in this PR?

# Given array types are different
pa.chunked_array([
pa.array([1, 2, 3]),
pa.array([1., 2., 3.])
])

with pytest.raises(TypeError):
with pytest.raises(pa.ArrowInvalid, match=msg):
Copy link
Member

Choose a reason for hiding this comment

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

(same)

@milesgranger milesgranger changed the title ARROW-15277: [Python] Use ChunkedArray::Make for chunked_array ARROW-15277: [C++][Python] Use ChunkedArray::Make for chunked_array Aug 24, 2022
@milesgranger
Copy link
Contributor Author

I don't think the R / AMD64 Windows R 3.6 RTools 35 failure is related...

@pitrou
Copy link
Member

pitrou commented Aug 25, 2022

Indeed, it isn't.

@pitrou
Copy link
Member

pitrou commented Aug 25, 2022

Rebased and a nit.

Copy link
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.

Thanks @milesgranger !

@pitrou
Copy link
Member

pitrou commented Aug 25, 2022

Python CI was successful here: https://github.com/milesgranger/arrow/actions/runs/2924730791

@pitrou pitrou merged commit dd0988b into apache:master Aug 25, 2022
@milesgranger milesgranger deleted the ARROW-15277_Use-Make-to-create-ChunkedArray branch August 25, 2022 08:54
@ursabot
Copy link

ursabot commented Aug 25, 2022

Benchmark runs are scheduled for baseline = 897c186 and contender = dd0988b. dd0988b is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Failed ⬇️0.58% ⬆️0.0%] test-mac-arm
[Failed ⬇️0.0% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.39% ⬆️0.07%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] dd0988b4 ec2-t3-xlarge-us-east-2
[Failed] dd0988b4 test-mac-arm
[Failed] dd0988b4 ursa-i9-9960x
[Finished] dd0988b4 ursa-thinkcentre-m75q
[Finished] 897c186f ec2-t3-xlarge-us-east-2
[Failed] 897c186f test-mac-arm
[Failed] 897c186f ursa-i9-9960x
[Finished] 897c186f ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

anjakefala pushed a commit to anjakefala/arrow that referenced this pull request Aug 31, 2022
…pache#13950)

Supersedes and will close apache#12096 

[ARROW-15277](https://issues.apache.org/jira/browse/ARROW-15277)

Lead-authored-by: Miles Granger <[email protected]>
Co-authored-by: Eduardo Ponce <[email protected]>
Co-authored-by: Antoine Pitrou <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
zagto pushed a commit to zagto/arrow that referenced this pull request Oct 7, 2022
…pache#13950)

Supersedes and will close apache#12096 

[ARROW-15277](https://issues.apache.org/jira/browse/ARROW-15277)

Lead-authored-by: Miles Granger <[email protected]>
Co-authored-by: Eduardo Ponce <[email protected]>
Co-authored-by: Antoine Pitrou <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
fatemehp pushed a commit to fatemehp/arrow that referenced this pull request Oct 17, 2022
…pache#13950)

Supersedes and will close apache#12096 

[ARROW-15277](https://issues.apache.org/jira/browse/ARROW-15277)

Lead-authored-by: Miles Granger <[email protected]>
Co-authored-by: Eduardo Ponce <[email protected]>
Co-authored-by: Antoine Pitrou <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
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.

4 participants