-
Notifications
You must be signed in to change notification settings - Fork 118
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
feat: bump narwhals and adapt to support pyarrow #694
Conversation
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.
Just one comment. If the comment clears it looks good to me!
Oh, and one more thing, must we bump narwhals? I usually prefer not to force the user to use the latest and greatest dependency, but it seems like we need it to support more dataframe types? |
Sadly yes, if we want to have pyarrow support for grouped meta and shift, these functionalities made it only in the latest release (yet narwhals keeps being dependency-free) |
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.
LGTM! Might be good to prep another release?
Sounds good! We went a bit silent, but now have some breaking changes from #693 |
Description
Long story short: I knew that some functionalities would have not worked for pyarrow even if using narwhals and we had to adjust accordingly.
For example we cannot create a series using
native_namespace.Series([...])
, because pyarrow doesn't have aSeries
object, therefore the a workaround was needed:Namely, create a narhwals dataframe for the given namespace and then select the unique column.
A bump of other changes were needed in the tests to assess with pyarrow tables.
Type of change
Checklist:
Other comments