You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/07/14 19:01:37 UTC
[GitHub] [arrow-datafusion] alamb opened a new pull request, #2914: Move `ScalarValue` tests alongside implementation, move `from_slice` to `datafusion_core`
alamb opened a new pull request, #2914:
URL: https://github.com/apache/arrow-datafusion/pull/2914
# Which issue does this PR close?
Closes https://github.com/apache/arrow-datafusion/issues/2913
# Rationale for this change
I am working on https://github.com/apache/arrow-datafusion/pull/2891 and it was not clear there were existing `ScalarValue` tests because they were in a different module than their implementation
Implementation: https://github.com/apache/arrow-datafusion/blob/fd64e6f2e5ca5ba8c33c431b75a3ba9441091970/datafusion/common/src/scalar.rs
Tests https://github.com/apache/arrow-datafusion/blob/85ca8be669dc1fbba4b2eefbf99fc2ed1546cc0c/datafusion/core/src/scalar.rs
🤯
# What changes are included in this PR?
1. Move `ScalarValue` tests alongside implementation
2. move `from_slice` to `datafusion_core` (as it is used in the ScalarValue implementation)
# Are there any user-facing changes?
`from_slice` is now in the `datafusion_core` crate (used to be in `datafusion_physical_expr` crate), but their is a `pub use` so no existing code needs to be changed (they can still import `from_slice` from `datafusion_physical_expr`)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb merged pull request #2914: Move `ScalarValue` tests alongside implementation, move `from_slice` to `datafusion_core`
Posted by GitBox <gi...@apache.org>.
alamb merged PR #2914:
URL: https://github.com/apache/arrow-datafusion/pull/2914
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] ursabot commented on pull request #2914: Move `ScalarValue` tests alongside implementation, move `from_slice` to `datafusion_core`
Posted by GitBox <gi...@apache.org>.
ursabot commented on PR #2914:
URL: https://github.com/apache/arrow-datafusion/pull/2914#issuecomment-1185440261
Benchmark runs are scheduled for baseline = fb2221c43d3367e876430e687ad6f1783cb79075 and contender = d8a2f5827a86941e6626b4ee7631801abe65b6f7. d8a2f5827a86941e6626b4ee7631801abe65b6f7 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on ec2-t3-xlarge-us-east-2] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/70fe02775a18489c902b54b9c6e858fb...a771a6e1f0ad4850b248a2d805faf35b/)
[Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on test-mac-arm] [test-mac-arm](https://conbench.ursa.dev/compare/runs/b6b05b3625a748988cb6c7f01e391cce...3cff5ca3cf544ffd9161cdf0f929b6e6/)
[Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on ursa-i9-9960x] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/6f91e158ffe144d6a9c888bf15541f3a...5d40642f932a43f1ae4f08f5e3c446f8/)
[Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on ursa-thinkcentre-m75q] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/93c89d2082784dcb991339217d6a8fc5...adc6189fccd145c4bd38147b4251e116/)
Buildkite builds:
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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #2914: Move `ScalarValue` tests alongside implementation, move `from_slice` to `datafusion_core`
Posted by GitBox <gi...@apache.org>.
alamb commented on code in PR #2914:
URL: https://github.com/apache/arrow-datafusion/pull/2914#discussion_r921482370
##########
datafusion/common/src/scalar.rs:
##########
@@ -1987,3 +1987,1176 @@ impl ScalarType<i64> for TimestampNanosecondType {
ScalarValue::TimestampNanosecond(r, None)
}
}
+
Review Comment:
these tests are moved, verbatim. The only change is that this line
```rust
use arrow::{array::*, datatypes::*};
```
was removed at the direction of the rust compiler as it was no longer used
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb commented on pull request #2914: Move `ScalarValue` tests alongside implementation, move `from_slice` to `datafusion_core`
Posted by GitBox <gi...@apache.org>.
alamb commented on PR #2914:
URL: https://github.com/apache/arrow-datafusion/pull/2914#issuecomment-1185437308
@comphead I merged this PR to make progress on other PRs. I will be happy to help rebase https://github.com/apache/arrow-datafusion/pull/2893 if you have trouble after this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb commented on pull request #2914: Move `ScalarValue` tests alongside implementation, move `from_slice` to `datafusion_core`
Posted by GitBox <gi...@apache.org>.
alamb commented on PR #2914:
URL: https://github.com/apache/arrow-datafusion/pull/2914#issuecomment-1184810547
cc @comphead this may affect https://github.com/apache/arrow-datafusion/pull/2893
Would you like me to wait for that to be ready to merge this PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org