-
Notifications
You must be signed in to change notification settings - Fork 15
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
Change behavior of apply_dimension and reduce #111
Conversation
…oved `target_dimension` parameter.
What I'm also thinking about is sorting, for example. It's just the values that change, but also reference system etc should stay intact. What I'm no wondering about is: Should the dimension labels be preserved when sorting? What about preserving the labels when computing the cumsum, for example? This is not really captured any longer with the "enumerated" labels. |
7dd1b4f
to
408d2bc
Compare
When sorting values, the quantity (variable) remains the same, but the labels change - you go from e.g. time axis When computing cumsum, the quantity changes (you go from value-at-t_i to sum-value-between-t_0-and-t_i); the labels stay the same, but they (might) change in meaning:
|
Ah, okay. So sort/order don't work well with apply_dimension now:
There's more work needed, also having in mind what @soxofaan wrote in #111 (comment) . Will give it a try and check how this can be streamlined better... (two processes either depending on label changes or cardinality change?) |
Co-Authored-By: Miha Kadunc <miha.kadunc@sinergise.com>
Telco: Agreed, but rename reduce to reduce_dimension. Edit: I guess, we should also rename filter to filer_labels then?! |
Updated the PR, I think we can merge with one or two reviews. I also renamed filter to filter_labels as it seems that's what is the direction also with reduce_dimension. Later, I need to check whether there are references to reduce and filter in API or the docs (e.g. the glossary) and update them. Remove waiting label from #94 once merged. |
…cesses in UDF processes.
To not further block my work, merging without approvals. If there's still something to change, open a new PR for it. |
Based on issue #73: Made apply_dimension more usable and made reduce more restricted to real "reduce" use cases.
apply_dimension
was basically replaced with a completely new definition.reduce
: Reducers can't return multiple values any longer. Parametertarget_dimension
was therefore removed.What do you think?