You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by namanmishra91 <gi...@git.apache.org> on 2017/12/05 13:21:22 UTC
[GitHub] zeppelin pull request #2696: [ZEPPELIN-3091] Correct aggregation functionali...
GitHub user namanmishra91 opened a pull request:
https://github.com/apache/zeppelin/pull/2696
[ZEPPELIN-3091] Correct aggregation functionality in charts
### What is this PR for?
The aggregation functions interpret NaN columns as 1 which leads to incorrect output being shown in charts. This PR fixes this by correcting the sum, min, max and average aggregation method.
### What type of PR is it?
Bug Fix
### Todos
### What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-3091
### How should this be tested?
* Update the data field of a paragraph results with %table to contain null values, e.g.
`"data": "age\tvalue\n19\t4\n20\t3\n21\t7\n22\t9\n23\t20\n24\t24\n25\t44\n26\t77\n27\t94\n28\t103\n29\t97\n20\t5\n20\tnull\n"`
The "null" values should be ignored for sum, min, max but included for count (and hence average).
### Screenshots (if appropriate)
### Questions:
* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/namanmishra91/zeppelin ZEPPELIN-3091
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zeppelin/pull/2696.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #2696
----
commit 568ae3f2ab5d63b088c7f625bacfee71ebbda10d
Author: Naman Mishra <na...@gmail.com>
Date: 2017-12-05T13:10:07Z
Correct aggregation functionality in charts
----
---
[GitHub] zeppelin issue #2696: [ZEPPELIN-3091] Correct aggregation functionality in c...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the issue:
https://github.com/apache/zeppelin/pull/2696
Thanks @namanmishra91. Looks great to me!
Merge to master if no further comment.
---
[GitHub] zeppelin issue #2696: [ZEPPELIN-3091] Correct aggregation functionality in c...
Posted by namanmishra91 <gi...@git.apache.org>.
Github user namanmishra91 commented on the issue:
https://github.com/apache/zeppelin/pull/2696
@Leemoonsoo Thanks. I have added a test case as you mentioned. Please have a look.
---
[GitHub] zeppelin issue #2696: [ZEPPELIN-3091] Correct aggregation functionality in c...
Posted by namanmishra91 <gi...@git.apache.org>.
Github user namanmishra91 commented on the issue:
https://github.com/apache/zeppelin/pull/2696
Ping
---
[GitHub] zeppelin pull request #2696: [ZEPPELIN-3091] Correct aggregation functionali...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/zeppelin/pull/2696
---
[GitHub] zeppelin issue #2696: [ZEPPELIN-3091] Correct aggregation functionality in c...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the issue:
https://github.com/apache/zeppelin/pull/2696
Thanks @namanmishra91 for important fix. Do you think you can add test case like https://github.com/apache/zeppelin/pull/2685, to ensure no future change break this aggregation behavior? Let me know if you need any help.
---