-
-
Notifications
You must be signed in to change notification settings - Fork 407
enh: Support selector and ds.summary for geom_aggregator
#6743
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
base: main
Are you sure you want to change the base?
Conversation
selector and ds.summary for geom_aggregator
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6743 +/- ##
==========================================
+ Coverage 87.83% 89.21% +1.37%
==========================================
Files 332 332
Lines 72010 72063 +53
==========================================
+ Hits 63250 64291 +1041
+ Misses 8760 7772 -988 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| if agg_state == AggState.AGG_BY: | ||
| params['vdims'] = list(map(str, agg.coords[agg_fn.column].data)) | ||
| elif agg_state == AggState.AGG_SEL_BY: | ||
| params['vdims'] = [d for d in agg.data_vars if d not in agg.attrs["selector_columns"]] |
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.
This is duplicated, but I think it is OK.
f80227a to
c7d6e9b
Compare
b46191b to
54936b9
Compare
54936b9 to
e259826
Compare
Resolves #6736
Resolves #6739
Mostly factoring out logic in
aggregateto also use forgeom_aggregate. Not sure if I like the new method name, but just wanted to push this before it was lost in a stash. Know at least one test is failing because of change ofNdOverlaytoImageStack.Summary
Screencast.From.2025-11-24.19-06-59.mp4
Selector
Screencast.From.2025-11-24.19-07-53.mp4
Simple
Screencast.From.2025-11-24.19-08-25.mp4