Add abs sum as an aggregator #1031
Conversation
|
Thank you for your contribution and Welcome to our Open Source Community! To make sure your pull request is accepted successfully, we ask all our open source contributors to sign a Contributor License Agreement. Having reviewed our contributor list, we require a CLA for the following people : (@harsh-jindal). If you need help obtaining a CLA, please read the Requirements for Contributions section of our CLA wiki or email help@finos.org with your questions. Thanks once again for your contribution. Let us work with you to make the CLA process quick, easy and efficient so we can move forward with reviewing and accepting your pull request. cc @finos-admin |
harsh-jindal
force-pushed
the
feature-abs-sum
branch
from
c57804e to
2355865
Compare
|
Thank you for your contribution and Welcome to our Open Source Community! To make sure your pull request is accepted successfully, we ask all our open source contributors to sign a Contributor License Agreement. Having reviewed our contributor list, we require a CLA for the following people : (@harsh-jindal). If you need help obtaining a CLA, please read the Requirements for Contributions section of our CLA wiki or email help@finos.org with your questions. Thanks once again for your contribution. Let us work with you to make the CLA process quick, easy and efficient so we can move forward with reviewing and accepting your pull request. cc @finos-admin |
harsh-jindal
force-pushed
the
feature-abs-sum
branch
from
2355865 to
4c25d1f
Compare
There was a problem hiding this comment.
Thanks for the PR! This is a great start!
I think we can merge this as is! It's not quite a complete fix for #750 yet, as we still need a couple changes to finish this out:
-
@finos/perspective-viewerJavascript module does not auto-export these enums from C++. This is just some technical debt from the project we have not addressed yet - there is no reason these values cannot be exported with some refactoring. However, as written, this new aggregate will not be available in the<perspective-viewer>web component! The available aggregates are grouped by type and set here. -
Tests! It's important for a project like Perspective to have good test coverage as its not really feasible to manually verify that functionality does not regress between releases. Here is a good example of what a single aggregate test looks like; you can run the test suite from your dev environment with
yarn testdetailed here.
I'm going to leave #750 open for now, but will merge this now. Thanks again for the great contribution!
|
I am new to open source community, I would like to work on this if it is really open? |
Adding 2 tests for abs sum function, this fixes finos#1031
Adding abs sum to <perspective-viewer>, this fixes finos#1031
Fixes #750