Skip to content

Conversation

@sanjibansg
Copy link
Contributor

This PR adds the feature to include the filename of a fragment if it is a FileFragment as a scalar value and includes the augmented fields in the dataset schema

@github-actions
Copy link

github-actions bot commented Mar 4, 2022

@sanjibansg sanjibansg marked this pull request as ready for review March 10, 2022 19:51
@sanjibansg sanjibansg requested a review from lidavidm March 14, 2022 04:24
Comment on lines 257 to 258
Copy link
Member

Choose a reason for hiding this comment

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

C++ may return an error, but Python will raise an exception, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I think so, I checked that again.
If we see here
https://github.com/apache/arrow/blob/master/python/pyarrow/_dataset.pyx#L2014

then this goes to here,
https://github.com/apache/arrow/blob/master/python/pyarrow/error.pxi#L99
which raises an ArrowInvalid exception. So, it might be better to mention that it raises an exception here.

cc: @westonpace

Copy link
Member

Choose a reason for hiding this comment

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

Yikes, yes this was totally my fault. My original suggestion was bad. Thank you for switching it back.

Copy link
Member

@lidavidm lidavidm left a comment

Choose a reason for hiding this comment

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

Thanks, just a minor suggestion

@sanjibansg sanjibansg requested a review from lidavidm March 30, 2022 14:53
@westonpace
Copy link
Member

Failure in hash-join-node-test is unrelated, but interesting. I've added it to ARROW-15221.

@ursabot
Copy link

ursabot commented Mar 31, 2022

Benchmark runs are scheduled for baseline = 600288e and contender = 3eb1bac. 3eb1bac 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.08% ⬆️0.0%] test-mac-arm
[Finished ⬇️0.0% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.04% ⬆️0.0%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/423| 3eb1bac0 ec2-t3-xlarge-us-east-2>
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/408| 3eb1bac0 test-mac-arm>
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/408| 3eb1bac0 ursa-i9-9960x>
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/418| 3eb1bac0 ursa-thinkcentre-m75q>
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/422| 600288e3 ec2-t3-xlarge-us-east-2>
[Failed] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/407| 600288e3 test-mac-arm>
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/407| 600288e3 ursa-i9-9960x>
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/417| 600288e3 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

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