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

Fix generate_interval_case in integration test #1446

Merged
merged 2 commits into from
Mar 17, 2022

Conversation

viirya
Copy link
Member

@viirya viirya commented Mar 15, 2022

Which issue does this PR close?

Closes #1445.

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

@github-actions github-actions bot added the arrow Changes to the arrow crate label Mar 15, 2022
@viirya
Copy link
Member Author

viirya commented Mar 15, 2022

There are some test failures. I will take look.

@codecov-commenter
Copy link

Codecov Report

Merging #1446 (5e4c8c3) into master (717216f) will decrease coverage by 0.02%.
The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1446      +/-   ##
==========================================
- Coverage   82.67%   82.65%   -0.03%     
==========================================
  Files         185      185              
  Lines       53866    53898      +32     
==========================================
+ Hits        44535    44549      +14     
- Misses       9331     9349      +18     
Impacted Files Coverage Δ
integration-testing/src/lib.rs 0.00% <0.00%> (ø)
arrow/src/compute/kernels/cast.rs 95.59% <100.00%> (+0.03%) ⬆️
arrow/src/datatypes/field.rs 53.79% <0.00%> (-0.31%) ⬇️
parquet/src/encodings/encoding.rs 93.37% <0.00%> (-0.19%) ⬇️
arrow/src/array/transform/mod.rs 86.31% <0.00%> (-0.12%) ⬇️
parquet_derive/src/parquet_field.rs 66.21% <0.00%> (+0.22%) ⬆️

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 717216f...5e4c8c3. 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.

Looks good to me -- thanks @viirya

@@ -228,6 +228,20 @@ pub fn can_cast_types(from_type: &DataType, to_type: &DataType) -> bool {
IntervalUnit::DayTime => true,
IntervalUnit::MonthDayNano => false, // Native type is i128
}
},
(Int32, Interval(to_type)) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

Choose a reason for hiding this comment

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

I double checked the format at this looks right to me

@alamb alamb merged commit b882d45 into apache:master Mar 17, 2022
@viirya
Copy link
Member Author

viirya commented Mar 17, 2022

Thank you @alamb !

@viirya viirya deleted the fix_generate_interval_case branch March 17, 2022 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrow Changes to the arrow crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix generate_interval_case integration test failure
3 participants