-
-
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(python): Selectors by_name
and by_dtype
should allow empty list as input
#11024
fix(python): Selectors by_name
and by_dtype
should allow empty list as input
#11024
Conversation
Hmm... not quite sure about this; I can see why it would work for this case, but may need some other/related tweaks if we want this behaviour. I'll be able to review it properly tomorrow night though 👍 |
Thanks! Happy to try to improve this if you think this could be useful! |
Hi, I just had another look at the code of the selectors and Looks like the empty list is fully covered in F.col so I think it shouldn't be an issue. It looks every case is controlled in that function. This could increase the consistency across other selectors too like datetime() (it uses a list to create the selector instead of an unpacked iterable), and consistency with the use of different ones as Thanks a lot! |
Apologies, got swamped by a few things; will get this sorted out |
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.
I looked into this, and it's a good change 👍 there's no need to unpack the lists here. Thanks!
by_name
and by_dtype
should allow empty list as input
…s input (pola-rs#11024) Co-authored-by: Antonio Caballero <antonio.caballero@concirrus.com>
…s input (pola-rs#11024) Co-authored-by: Antonio Caballero <antonio.caballero@concirrus.com>
Closes #10855
Currently the selectors are being created with an unpacked list
F.col(*all_names)
, but sinceF.col
can manage iterables, by not unpacking we allow selecting nothing by inputing an empty list.Fixing this in my opinion gives more consistency on how some functions work:
with_columns
also works with empty lists for example.