-
-
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
fix: zip_with also broadcast mask #12309
Conversation
Well... That was easy! 😄 |
@reswqa shall we take this opportunity to write it via |
@ritchie46 As for the When I tried to do this, I ran into lifecycle issues with Take the following as example: (l_len, 1, mask_len) if l_len == mask_len => {
let right = $other.get(0);
let val:ChunkedArray<$ty> = binary_elementwise($mask, $self, |mask:Option<bool>, left|{
ternary_apply(mask.unwrap_or(false), left, right)
}).with_name($self.name());
Ok(val)
}, error: implementation of `FnOnce` is not general enough
--> crates/polars-core/src/chunked_array/ops/zip.rs:76:41
|
76 | let val:ChunkedArray<$ty> = binary_elementwise($mask, $self, |mask:Option<bool>, left|{
| _________________________________________^
77 | | ternary_apply(mask.unwrap_or(false), left, right)
78 | | }).with_name($self.name());
| |_______________^ implementation of `FnOnce` is not general enough
...
174 | / impl_ternary_broadcast!(
175 | | self,
176 | | self.len(),
177 | | other.len(),
... |
183 | | )
| |_____________- in this macro invocation
|
= note: closure with signature `fn(std::option::Option<bool>, std::option::Option<&'2 [u8]>) -> std::option::Option<&[u8]>` must implement `FnOnce<(std::option::Option<bool>, std::option::Option<&'1 [u8]>)>`, for any lifetime `'1`...
= note: ...but it actually implements `FnOnce<(std::option::Option<bool>, std::option::Option<&'2 [u8]>)>`, for some specific lifetime `'2` |
I think that's the same issue I ran into. @orlp showed me how to fix: #12071 (comment) |
@reswqa I think we can follow up with the improvement on a second PR. This PR is the fix. Would love to see if we can make it better with proper column traversal. :) |
Per discuss in discord, and It seems that all test can be passed.