Skip to content
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 issue 976 #980

Merged
merged 7 commits into from
Sep 9, 2024
Merged

Fix issue 976 #980

merged 7 commits into from
Sep 9, 2024

Conversation

billylanchantin
Copy link
Member

@billylanchantin billylanchantin commented Sep 7, 2024

Description

Fixes an issue with DF.new where a well placed nil could result in a panic.

Basically, the work was being done by Native.s_from_list_of_series/2 which attempted to do following:

  • create a list of individual polars_core::seriess each with the same dtype
  • create a final polars_core::series of type {:list, dtype} from that list

But because s_from_list_of_series/2 didn't have the final dtype information up front, it was possible for two elements of the intermediate list to have incompatible dtypes. When this happened, the final creation would panic.

Changes

This PR makes the dtype a required argument for:

  • s_from_list_of_series/3
  • s_from_list_of_series_as_structs/3

(Those two seemed like a pair so I made them the same.)

Links

Discussion

I think there's an opportunity for optimization since we're now passing the expected dtype, but I didn't explore it.

Copy link
Contributor

@philss philss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@billylanchantin billylanchantin merged commit ffcb2e6 into main Sep 9, 2024
4 checks passed
@billylanchantin billylanchantin deleted the bl-fix-issue-976 branch September 9, 2024 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Panic for list of lists with nil entries
3 participants