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.
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
add
method
andweight
feature to (region) aggregation #305add
method
andweight
feature to (region) aggregation #305Changes from 22 commits
10cf203
85afde4
08b894a
4636f90
c22dca0
7ef3516
9969c74
6dc3228
978829b
bf5813c
02adebc
4a5e359
fa152e2
4e7fdc2
92b292c
08ce709
55f21c0
14de79f
a3901a1
9688aa1
63a5d8a
ba71cc7
2d50535
c582003
b727cee
ee630aa
01637b7
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
if this is for weighted average, then surely method is always sum/np.sum? or is this just meant as a placeholder for future methods?
Consider also instead using
np.nansum
such that if a value is missing / nan, the calculation won't returnnan
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.
The
method
arg is partly anticipating more supported functions in the future, partly for checking that the chosen method is indeed sum in that function (moving some not-so important stuff to lower-level functions, also for reusing_aggregate_weight()
in other functions in the future and not needing to implement that check in multiple higher-level functions.The
data
table cannot havenan
(they are removed at initialisation), so this point is moot. If a data value or the weight value is missing (i.e., inconsistent series index's), aValueError
is raised.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.
would you put
.agg(method)
here instead of.sum()
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.
do you also want to limit inputs to the
KNOWN_FUNCS
?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.
only summation is allowed (for the time being) anyway, so imho no need to be more complicated...
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.
Agree that we don't need anything more complicated here, but maybe worth putting in some comments about what would need to be changed in the future if more than sum is supported?
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.
maybe add a
'mean'
synonym as well?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.
done