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

Add datafusion::test_util, resolve test data paths without env vars #498

Merged
merged 1 commit into from
Jun 4, 2021

Conversation

mluts
Copy link
Contributor

@mluts mluts commented Jun 3, 2021

Which issue does this PR close?

Closes #467

Rationale for this change

What changes are included in this PR?

Just ported the code from apache/arrow-rs

Are there any user-facing changes?

Simplified "Examples" section & running tests 🙂

@codecov-commenter
Copy link

codecov-commenter commented Jun 3, 2021

Codecov Report

Merging #498 (c21f900) into master (01b57f7) will increase coverage by 0.12%.
The diff coverage is 80.88%.

❗ Current head c21f900 differs from pull request most recent head 177cdc0. Consider uploading reports for the commit 177cdc0 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #498      +/-   ##
==========================================
+ Coverage   75.84%   75.97%   +0.12%     
==========================================
  Files         153      155       +2     
  Lines       25876    26438     +562     
==========================================
+ Hits        19626    20086     +460     
- Misses       6250     6352     +102     
Impacted Files Coverage Δ
...sta/rust/core/src/serde/logical_plan/from_proto.rs 35.91% <0.00%> (-0.31%) ⬇️
...ta/rust/core/src/serde/physical_plan/from_proto.rs 38.65% <0.00%> (-1.13%) ⬇️
...ista/rust/core/src/serde/physical_plan/to_proto.rs 50.31% <0.00%> (-0.32%) ⬇️
datafusion/src/execution/context.rs 92.08% <ø> (ø)
datafusion/src/optimizer/utils.rs 47.51% <0.00%> (-2.49%) ⬇️
datafusion/src/physical_plan/functions.rs 92.70% <ø> (ø)
python/src/dataframe.rs 0.00% <0.00%> (ø)
python/src/expression.rs 0.00% <0.00%> (ø)
...lista/rust/core/src/serde/logical_plan/to_proto.rs 62.40% <15.38%> (ø)
datafusion/src/optimizer/hash_build_probe_order.rs 58.53% <20.00%> (-1.97%) ⬇️
... and 21 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 01b57f7...177cdc0. Read the comment docs.

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thanks @mluts ! I tried this out locally and it works well.

git clone git@github.com:mluts/arrow-datafusion.git
cd arrow-datafusion/
git checkout  resolve_test_data_paths 
unset PARQUET_TEST_DATA
unset ARROW_TEST_DATA
cargo run --example csv_sql
# failed, but told me to run following command
git submodule update --init
# passes!!
cargo run --example csv_sql

It looks like there are a few linting / test errors, but after that is fixed I think this PR is ready to go:

---- src\datasource\csv.rs - datasource::csv (line 24) stdout ----
error[E0433]: failed to resolve: unresolved import
 --> src\datasource\csv.rs:28:23
  |
7 | let testdata = crate::test_util::arrow_test_data();
  |                       ^^^^^^^^^
  |                       |
  |                       unresolved import
  |                       help: a similar path exists: `datafusion::test_util`

@mluts mluts force-pushed the resolve_test_data_paths branch 3 times, most recently from 075056a to 177cdc0 Compare June 4, 2021 13:16
@mluts
Copy link
Contributor Author

mluts commented Jun 4, 2021

Managed to fix all the checks, yay 🙂

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thanks @mluts looks good!

@alamb alamb merged commit c92079d into apache:master Jun 4, 2021
@houqp houqp added the datafusion Changes in the datafusion crate label Jul 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
datafusion Changes in the datafusion crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Automatically find PARQUET_TEST_DATA and ARROW_TEST_DATA
4 participants