Implement subset of split-apply-combine pattern#396
Open
ph-kev wants to merge 2 commits into
Open
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #396 +/- ##
==========================================
+ Coverage 99.46% 99.47% +0.01%
==========================================
Files 18 19 +1
Lines 2048 2096 +48
==========================================
+ Hits 2037 2085 +48
Misses 11 11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
9cc61e2 to
0691e4c
Compare
imreddyTeja
approved these changes
May 19, 2026
Member
imreddyTeja
left a comment
There was a problem hiding this comment.
This looks good. I like this design pattern, but I don't really understand what the combine part does.
2e5d3a8 to
4124042
Compare
nefrathenrici
approved these changes
May 21, 2026
4124042 to
d03b9d2
Compare
d03b9d2 to
03fc2ca
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #279
This PR implements a subset of the split-apply-combine pattern which are supporting reductions, single group split, and seasonal splits. This allows the users to be more expressive with the kind of reductions that they can do. The implementation of the split-apply-combine pattern also keep the dimension that is reduced over which can be helpful for calibration (e.g. certain calibration functionality such as
ObservationReciperequires a time dimension).Here's a list of concerns that I have for this PR:
AbstractApplyOperation.SplitApplyVar. This can be changed later without it being a breaking change.In the future, for the split operations, there would be support for arbitrary splits that the user can supply. Additionally, the apply operations can also include transformations (which keep the size of the array the same) and filter (requires adding a new dimension). An example of a transformation is doing a z-score transformation each group (e.g. seasons) and an example of a filter is getting the top k elements of each group. Both use cases are not needed right now, so they are not included in this PR.