-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
chore: Fix incorrect debug assertion in ChunkedArray::from_chunks_and_dtype
#16697
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #16697 +/- ##
==========================================
- Coverage 81.49% 81.48% -0.01%
==========================================
Files 1416 1416
Lines 186880 186880
Branches 3023 3023
==========================================
- Hits 152289 152279 -10
- Misses 34059 34070 +11
+ Partials 532 531 -1 ☔ View full report in Codecov by Sentry. |
@@ -206,8 +206,8 @@ where | |||
// that check if the data types in the arrays are as expected | |||
#[cfg(debug_assertions)] | |||
{ | |||
if !chunks.is_empty() && dtype.is_primitive() { | |||
assert_eq!(chunks[0].data_type(), &dtype.to_physical().to_arrow(true)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The .to_physical()
here is unneeded as we have already checked that dtype
is primitive.
CodSpeed Performance ReportMerging #16697 will not alter performanceComparing Summary
|
Converting an empty Array-of-Dates type resulted in a panic due to a debug assert:
Looks like empty arrays end up with the wrong data type here. I followed the trace but couldn't work out exactly why that happens. I did find this TODO comment which seems related:
polars/crates/polars-core/src/chunked_array/array/iterator.rs
Lines 41 to 43 in 9ea3bb6
For now I have disabled the debug assertion for empty arrays. It resolves this issue.