-
-
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(rust, python): handle concat_list with first lit value #7235
Conversation
let max_len = other.iter().map(|s| s.len()).max().unwrap(); | ||
if max_len > 1 { | ||
new_ca = first_ca.new_from_index(0, max_len); | ||
first_ca = &new_ca; |
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.
Is there a nicer Rust pattern for handling this reassignment? The compiler really wanted this new series to have a scope outside the if statement to let me reassign it.
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.
That's because new_ca
would get dropped when the scope ends.
You can clone on line 144. Then you can assign directly
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.
Ah! Thanks for that explanation. Still new to Rust. Does to_owned()
offer any benefit over clone()
there? I'm not sure how expensive a clone()
on a ChunkedArray
. Does it end up doing a deep copy of all it's items or just the wrapping type?
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.
Does it end up doing a deep copy of all it's items or just the wrapping type?
The data is wrapped by an Arc
so it is a cheap clone.
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.
Thanks!
let max_len = other.iter().map(|s| s.len()).max().unwrap(); | ||
if max_len > 1 { | ||
new_ca = first_ca.new_from_index(0, max_len); | ||
first_ca = &new_ca; |
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.
That's because new_ca
would get dropped when the scope ends.
You can clone on line 144. Then you can assign directly
The first value passed to
pl.concat_list
receives some special treatment being the lhs. This means that using apl.lit
or any unit series doesn't get expanded as expected. Using a unit value in any other position works as you'd expected.Example:
Related #7236
Tangentially related #6608