-
Notifications
You must be signed in to change notification settings - Fork 166
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
feat: add variance function #263
Conversation
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.
Same as for #257: not sure how to feel about the integer variants, and the floating point operations should use float rounding options rather than integer overflow options.
@jvanstraten I understand. We can hold until the decision is finalized. |
YAML CI:
@jacques-n I forgot which earlier PR this came up in. Could that have been one of the PRs you closed yesterday, maybe? I figured it'd be in main already. |
470ad7c
to
485d3c5
Compare
@jvanstraten I updated the PR. |
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.
LGTM aside from:
@jvanstraten updated with the suggestion. |
Same CI issues as #257, but LGTM otherwise. |
ea82696
to
5e22f5d
Compare
@jvanstraten updated the PR, there was a typo. Let's check the CIs once again. Sorry about this. |
5e22f5d
to
ebbc08f
Compare
@jvanstraten I squashed the commits to a single one. Let's see the CIs. |
Hey @vibhatha , looks like other merges caused a merge conflict here. Can you rebase/fix? Thanks! |
@jacques-n shall we go with this: #257 first and I will resolve conflicts after this. |
#257 is merged now. |
06aa72a
to
c7f6ca7
Compare
@jacques-n could you please check the content after rebase. |
This PR includes the addition of variance aggregation operator.