You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zeppelin.apache.org by mo...@apache.org on 2017/12/14 16:39:17 UTC
zeppelin git commit: [ZEPPELIN-3091] Correct aggregation
functionality in charts
Repository: zeppelin
Updated Branches:
refs/heads/master 971631c1a -> f8cd64cb5
[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
Author: Naman Mishra <na...@gmail.com>
Closes #2696 from namanmishra91/ZEPPELIN-3091 and squashes the following commits:
d8a57c238 [Naman Mishra] Add test
38ad39c65 [Naman Mishra] Merge branch 'master' into ZEPPELIN-3091
568ae3f2a [Naman Mishra] Correct aggregation functionality in charts
Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo
Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/f8cd64cb
Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/f8cd64cb
Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/f8cd64cb
Branch: refs/heads/master
Commit: f8cd64cb50028031ab321144697d98822a60c63f
Parents: 971631c
Author: Naman Mishra <na...@gmail.com>
Authored: Tue Dec 12 10:45:34 2017 +0530
Committer: Lee moon soo <mo...@apache.org>
Committed: Thu Dec 14 08:39:14 2017 -0800
----------------------------------------------------------------------
zeppelin-web/src/app/tabledata/pivot.js | 36 +++++++++++-----
.../src/app/tabledata/tabledata.test.js | 45 ++++++++++++++++++++
2 files changed, 71 insertions(+), 10 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/f8cd64cb/zeppelin-web/src/app/tabledata/pivot.js
----------------------------------------------------------------------
diff --git a/zeppelin-web/src/app/tabledata/pivot.js b/zeppelin-web/src/app/tabledata/pivot.js
index a0f61b2..da29900 100644
--- a/zeppelin-web/src/app/tabledata/pivot.js
+++ b/zeppelin-web/src/app/tabledata/pivot.js
@@ -138,8 +138,8 @@ export default class PivotTransformation extends Transformation {
pivot (data, keys, groups, values) {
let aggrFunc = {
sum: function (a, b) {
- let varA = (a !== undefined) ? (isNaN(a) ? 1 : parseFloat(a)) : 0
- let varB = (b !== undefined) ? (isNaN(b) ? 1 : parseFloat(b)) : 0
+ let varA = (a !== undefined) ? (isNaN(a) ? 0 : parseFloat(a)) : 0
+ let varB = (b !== undefined) ? (isNaN(b) ? 0 : parseFloat(b)) : 0
return varA + varB
},
count: function (a, b) {
@@ -148,22 +148,38 @@ export default class PivotTransformation extends Transformation {
return varA + varB
},
min: function (a, b) {
- let varA = (a !== undefined) ? (isNaN(a) ? 1 : parseFloat(a)) : 0
- let varB = (b !== undefined) ? (isNaN(b) ? 1 : parseFloat(b)) : 0
- return Math.min(varA, varB)
+ let aIsValid = isValidNumber(a)
+ let bIsValid = isValidNumber(b)
+ if (!aIsValid) {
+ return parseFloat(b)
+ } else if (!bIsValid) {
+ return parseFloat(a)
+ } else {
+ return Math.min(parseFloat(a), parseFloat(b))
+ }
},
max: function (a, b) {
- let varA = (a !== undefined) ? (isNaN(a) ? 1 : parseFloat(a)) : 0
- let varB = (b !== undefined) ? (isNaN(b) ? 1 : parseFloat(b)) : 0
- return Math.max(varA, varB)
+ let aIsValid = isValidNumber(a)
+ let bIsValid = isValidNumber(b)
+ if (!aIsValid) {
+ return parseFloat(b)
+ } else if (!bIsValid) {
+ return parseFloat(a)
+ } else {
+ return Math.max(parseFloat(a), parseFloat(b))
+ }
},
avg: function (a, b, c) {
- let varA = (a !== undefined) ? (isNaN(a) ? 1 : parseFloat(a)) : 0
- let varB = (b !== undefined) ? (isNaN(b) ? 1 : parseFloat(b)) : 0
+ let varA = (a !== undefined) ? (isNaN(a) ? 0 : parseFloat(a)) : 0
+ let varB = (b !== undefined) ? (isNaN(b) ? 0 : parseFloat(b)) : 0
return varA + varB
}
}
+ let isValidNumber = function(num) {
+ return num !== undefined && !isNaN(num)
+ }
+
let aggrFuncDiv = {
sum: false,
count: false,
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/f8cd64cb/zeppelin-web/src/app/tabledata/tabledata.test.js
----------------------------------------------------------------------
diff --git a/zeppelin-web/src/app/tabledata/tabledata.test.js b/zeppelin-web/src/app/tabledata/tabledata.test.js
index 3de2fa3..e24b073 100644
--- a/zeppelin-web/src/app/tabledata/tabledata.test.js
+++ b/zeppelin-web/src/app/tabledata/tabledata.test.js
@@ -83,4 +83,49 @@ describe('PivotTransformation build', function() {
expect(config.common.pivot.keys[1].index).toBe(3)
expect(config.common.pivot.keys[2].index).toBe(5)
})
+
+ it('should aggregate values correctly', function() {
+ let td = new TableData()
+ td.loadParagraphResult({
+ type: 'TABLE',
+ msg: 'key\tvalue\na\t10\na\tnull\na\t0\na\t1\n'
+ })
+
+ let config = {
+ common: {
+ pivot: {
+ keys: [
+ {
+ 'name': 'key',
+ 'index': 0.0,
+ }
+ ],
+ groups: [],
+ values: [
+ {
+ 'name': 'value',
+ 'index': 1.0,
+ 'aggr': 'sum'
+ }
+ ]
+ }
+ }
+ }
+
+ pt.setConfig(config)
+ let transformed = pt.transform(td)
+ expect(transformed.rows['a']['value(sum)'].value).toBe(11)
+
+ pt.config.common.pivot.values[0].aggr = 'max'
+ transformed = pt.transform(td)
+ expect(transformed.rows['a']['value(max)'].value).toBe(10)
+
+ pt.config.common.pivot.values[0].aggr = 'min'
+ transformed = pt.transform(td)
+ expect(transformed.rows['a']['value(min)'].value).toBe(0)
+
+ pt.config.common.pivot.values[0].aggr = 'count'
+ transformed = pt.transform(td)
+ expect(transformed.rows['a']['value(count)'].value).toBe(4)
+ })
})