fix: Support Datetime broadcast in list.concat
#23137
Conversation
kdn36
requested review from
ritchie46,
c-peters,
alexander-beedie,
MarcoGorelli,
reswqa and
orlp
as code owners
github-actions
bot
added
fix
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #23137 +/- ##
=======================================
Coverage 80.84% 80.85%
=======================================
Files 1615 1615
Lines 217405 217418 +13
Branches 2809 2809
=======================================
+ Hits 175767 175793 +26
+ Misses 40974 40961 -13
Partials 664 664 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| .iter() | ||
| .flat_map(|s| { | ||
| let lst = s.list().unwrap(); | ||
| lst.get_as_series(0) | ||
| }) | ||
| .collect::<Vec<_>>(); | ||
|
|
||
| // we may need to recast back from physical dtype |
There was a problem hiding this comment.
nit: move this into the above loop?
There was a problem hiding this comment.
ok - as a conditional extra map or unconditional inside the flat_map? (unconditional sounds right and consistent, however it is unsafe)
There was a problem hiding this comment.
ok - as a conditional extra
mapor unconditional inside theflat_map? (unconditional sounds right and consistent, however it is unsafe)
I think this is up to your personal preference.
One other thing, can you as a drive-by change that to use filter_map instead of flat_map.
There was a problem hiding this comment.
ack & done - ended up keeping the to/from_physical calls close together
fixes #23102
Pending review, see issue for solution strategies.
Works for Datetime and Duration, but not for Decimal as cast behaves differently. Should we use
from_physical_uncheckedinstead?*edit: latest commit uses
from_physical_uncheckedand works for Decimal as well.