-
Notifications
You must be signed in to change notification settings - Fork 13
feat: Add dy.Array column type
#27
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
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #27 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 37 38 +1
Lines 1808 1845 +37
=========================================
+ Hits 1808 1845 +37 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
borchero
left a comment
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.
Thank you, this looks great already! I left a couple of small comments but I think it is almost ready :)
Great job finding all the right places to add code, esp. in the tests, and thanks for maintaining the 100% coverage 😁🚀
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.
Could you please also update the mypy plugin and tests/test_typing.py similar to #29? Thanks!
borchero
left a comment
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.
Looks good to me (modulo the mypy plugin that @delsner mentioned), thanks! 🚀
delsner
left a comment
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.
Great work, thanks! 🚀 I added a typing test and updated the mypy plugin code.
Motivation
Dataframely currently doesn't support the
pl.Arraycolumn type, and so dataframes using array columns cannot be validated. I'd like to be able to validate those, too.Changes
This PR implements the
dy.Arraycolumn type corresponding to polars'pl.Arraytype.The implementation is very similar to the
dy.List, however, polars List differs from Array mostly in the following ways:pl.Expr.arrhas less functionality compared topl.Expr.list, most notably there is no.arr.eval()function, which complicates validating rules. One option is to recursively convert the array column to nested lists just for validation, but since the performance would be suboptimal, I left this part unimplemented and only allowed Array's inner column types without any validation rules.Would be happy to hear any feedback!