feat: Add DataFrame support in dy.Collection#335
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #335 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 56 56
Lines 3404 3427 +23
=========================================
+ Hits 3404 3427 +23 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds first-class support for eager dy.DataFrame[...] members in dy.Collection, enabling collections to expose Polars DataFrames directly (instead of always returning LazyFrames).
Changes:
- Extend member metadata with an
is_lazyflag and addlazy_members()/eager_members()helpers. - Introduce internal
_to_lazy_dict()and switch internal operations (join/collect_all/storage/filter-result collection) to use it. - Update collection initialization to eagerly
.collect()members annotated asdy.DataFrame[...], plus add dedicated tests for eager/lazy/mixed collections.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
dataframely/collection/_base.py |
Adds MemberInfo.is_lazy, derives eager vs lazy from annotations, introduces _to_lazy_dict(), and updates _init() to materialize eager members. |
dataframely/collection/collection.py |
Routes internal operations (join/collect_all/storage) through _to_lazy_dict() to support mixed eager/lazy collections. |
dataframely/collection/filter_result.py |
Ensures CollectionFilterResult.collect_all() operates on lazy views of members for mixed collections. |
tests/collection/test_implementation.py |
Updates annotation implementation tests to reflect dy.DataFrame[...] support. |
tests/collection/test_dataframe_members.py |
Adds new end-to-end tests for eager-only, lazy-only, mixed, and optional eager members. |
Oliver Borchert (borchero)
left a comment
There was a problem hiding this comment.
Sorry it took so long to review this! Generally, I think that we can support this, I left a few comments to simplify the implementation a little
Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: quant-ranger[bot] <132915763+quant-ranger[bot]@users.noreply.github.com> Co-authored-by: Oliver Borchert <oliver.borchert@quantco.com>
Co-authored-by: Oliver Borchert <me@borchero.com>
Co-authored-by: quant-ranger[bot] <132915763+quant-ranger[bot]@users.noreply.github.com> Co-authored-by: Oliver Borchert <oliver.borchert@quantco.com>
04018ab to
4bf6d8b
Compare
Oliver Borchert (borchero)
left a comment
There was a problem hiding this comment.
Thanks for the updates gab23r!
Motivation
Closes #319
Changes
is_lazyattribute inMemberInfo.lazy_membersandeager_memberstody.Collection-_to_lazy_dictmethods.Collection._initto collect the dataframe is the eager case.