From 6309837f46ddca9369d9d5624a9d7e1918899dcf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 26 Sep 2016 14:38:34 -0400 Subject: [PATCH 01/26] fix linting in lib/filter.js --- lib/filter.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/filter.js b/lib/filter.js index e7e4ecf078e..cb0f9ba8fad 100644 --- a/lib/filter.js +++ b/lib/filter.js @@ -6,4 +6,6 @@ * LICENSE file in the root directory of this source tree. */ +'use strict'; + module.exports = require('../src/transforms/filter'); From 96bcbfcc7eb57ba7335c35e137a8e45daffb7ca7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 26 Sep 2016 14:38:56 -0400 Subject: [PATCH 02/26] lib: add groupby module + register it in main index --- lib/groupby.js | 11 +++++++++++ lib/index.js | 6 ++++-- 2 files changed, 15 insertions(+), 2 deletions(-) create mode 100644 lib/groupby.js diff --git a/lib/groupby.js b/lib/groupby.js new file mode 100644 index 00000000000..fc7bc528e09 --- /dev/null +++ b/lib/groupby.js @@ -0,0 +1,11 @@ +/** +* Copyright 2012-2016, Plotly, Inc. +* All rights reserved. +* +* This source code is licensed under the MIT license found in the +* LICENSE file in the root directory of this source tree. +*/ + +'use strict'; + +module.exports = require('../src/transforms/groupby'); diff --git a/lib/index.js b/lib/index.js index 7f767ebe8e1..177c72d6102 100644 --- a/lib/index.js +++ b/lib/index.js @@ -10,6 +10,7 @@ var Plotly = require('./core'); +// traces Plotly.register([ require('./bar'), require('./box'), @@ -30,9 +31,10 @@ Plotly.register([ require('./scattermapbox') ]); -// add transforms +// transforms Plotly.register([ - require('./filter') + require('./filter'), + require('./groupby') ]); module.exports = Plotly; From 06c805d6da4f3bea05dc71c9e458a072ea32aec0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 26 Sep 2016 14:39:22 -0400 Subject: [PATCH 03/26] lint: rm useless comments in transform modules --- src/transforms/filter.js | 14 ++------------ src/transforms/groupby.js | 9 +-------- 2 files changed, 3 insertions(+), 20 deletions(-) diff --git a/src/transforms/filter.js b/src/transforms/filter.js index 9bbe2700f57..d54e7358c1a 100644 --- a/src/transforms/filter.js +++ b/src/transforms/filter.js @@ -10,18 +10,12 @@ var isNumeric = require('fast-isnumeric'); -// var Lib = require('@src/lib'); var Lib = require('../lib'); -/* eslint no-unused-vars: 0*/ - -// so that Plotly.register knows what to do with it exports.moduleType = 'transform'; -// determines to link between transform type and transform module exports.name = 'filter'; -// ... as trace attributes exports.attributes = { operation: { valType: 'enumerated', @@ -35,11 +29,7 @@ exports.attributes = { filtersrc: { valType: 'enumerated', values: ['x', 'y', 'ids'], - dflt: 'x', - ids: { - valType: 'data_array', - description: 'A list of keys for object constancy of data points during animation' - } + dflt: 'x' } }; @@ -56,7 +46,7 @@ exports.attributes = { * @return {object} transformOut * copy of transformIn that contains attribute defaults */ -exports.supplyDefaults = function(transformIn, fullData, layout) { +exports.supplyDefaults = function(transformIn) { var transformOut = {}; function coerce(attr, dflt) { diff --git a/src/transforms/groupby.js b/src/transforms/groupby.js index a2a9e97816b..7838b6e76fc 100644 --- a/src/transforms/groupby.js +++ b/src/transforms/groupby.js @@ -8,19 +8,12 @@ 'use strict'; -// var Lib = require('@src/lib'); var Lib = require('../lib'); -/* eslint no-unused-vars: 0*/ - - -// so that Plotly.register knows what to do with it exports.moduleType = 'transform'; -// determines to link between transform type and transform module exports.name = 'groupby'; -// ... as trace attributes exports.attributes = { active: { valType: 'boolean', @@ -49,7 +42,7 @@ exports.attributes = { * @return {object} transformOut * copy of transformIn that contains attribute defaults */ -exports.supplyDefaults = function(transformIn, fullData, layout) { +exports.supplyDefaults = function(transformIn) { var transformOut = {}; function coerce(attr, dflt) { From f6eb2f40a34d7a9086352bc735e80eb93a7582c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 26 Sep 2016 14:39:47 -0400 Subject: [PATCH 04/26] lint: rm uselss Plotly.register calls in test suites --- test/jasmine/tests/gl2d_pointcloud_test.js | 5 ----- test/jasmine/tests/plotschema_test.js | 5 ----- test/jasmine/tests/transform_filter_test.js | 3 --- test/jasmine/tests/transform_groupby_test.js | 3 --- test/jasmine/tests/transform_multi_test.js | 5 ----- test/jasmine/tests/validate_test.js | 4 ---- 6 files changed, 25 deletions(-) diff --git a/test/jasmine/tests/gl2d_pointcloud_test.js b/test/jasmine/tests/gl2d_pointcloud_test.js index 57773c86477..6cc0017ad21 100644 --- a/test/jasmine/tests/gl2d_pointcloud_test.js +++ b/test/jasmine/tests/gl2d_pointcloud_test.js @@ -2,11 +2,6 @@ var Plotly = require('@lib/index'); -// pointcloud is not part of the dist plotly.js bundle initially -Plotly.register([ - require('@lib/pointcloud') -]); - // Test utilities var createGraphDiv = require('../assets/create_graph_div'); var destroyGraphDiv = require('../assets/destroy_graph_div'); diff --git a/test/jasmine/tests/plotschema_test.js b/test/jasmine/tests/plotschema_test.js index ffe72a93ccc..013db8d2279 100644 --- a/test/jasmine/tests/plotschema_test.js +++ b/test/jasmine/tests/plotschema_test.js @@ -1,11 +1,6 @@ var Plotly = require('@lib/index'); var Lib = require('@src/lib'); -Plotly.register([ - require('@src/transforms/filter'), - require('@src/transforms/groupby') -]); - describe('plot schema', function() { 'use strict'; diff --git a/test/jasmine/tests/transform_filter_test.js b/test/jasmine/tests/transform_filter_test.js index 4e7bed11757..25aa7684f95 100644 --- a/test/jasmine/tests/transform_filter_test.js +++ b/test/jasmine/tests/transform_filter_test.js @@ -7,9 +7,6 @@ var destroyGraphDiv = require('../assets/destroy_graph_div'); var assertDims = require('../assets/assert_dims'); var assertStyle = require('../assets/assert_style'); -Plotly.register([ - require('@src/transforms/filter') -]); describe('one-to-one transforms:', function() { 'use strict'; diff --git a/test/jasmine/tests/transform_groupby_test.js b/test/jasmine/tests/transform_groupby_test.js index d37ef142d25..bb2ea0f607e 100644 --- a/test/jasmine/tests/transform_groupby_test.js +++ b/test/jasmine/tests/transform_groupby_test.js @@ -6,9 +6,6 @@ var destroyGraphDiv = require('../assets/destroy_graph_div'); var assertDims = require('../assets/assert_dims'); var assertStyle = require('../assets/assert_style'); -Plotly.register([ - require('@src/transforms/groupby') -]); describe('groupby', function() { diff --git a/test/jasmine/tests/transform_multi_test.js b/test/jasmine/tests/transform_multi_test.js index ae1b7abd94b..0b63c2c8e5e 100644 --- a/test/jasmine/tests/transform_multi_test.js +++ b/test/jasmine/tests/transform_multi_test.js @@ -7,11 +7,6 @@ var assertDims = require('../assets/assert_dims'); var assertStyle = require('../assets/assert_style'); -Plotly.register([ - require('@src/transforms/filter'), - require('@src/transforms/groupby') -]); - describe('multiple transforms:', function() { 'use strict'; diff --git a/test/jasmine/tests/validate_test.js b/test/jasmine/tests/validate_test.js index 4d6655c957a..10771bc154a 100644 --- a/test/jasmine/tests/validate_test.js +++ b/test/jasmine/tests/validate_test.js @@ -1,10 +1,6 @@ var Plotly = require('@lib/index'); var Lib = require('@src/lib'); -Plotly.register([ - require('@src/transforms/filter'), - require('@src/transforms/groupby') -]); describe('Plotly.validate', function() { From 0158f45ae0340db0c8b810be40d48e473242704c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 26 Sep 2016 14:57:26 -0400 Subject: [PATCH 05/26] test: move user-defined transform test to transform_multi --- test/jasmine/tests/transform_filter_test.js | 39 ------------------ test/jasmine/tests/transform_multi_test.js | 45 +++++++++++++++++++++ 2 files changed, 45 insertions(+), 39 deletions(-) diff --git a/test/jasmine/tests/transform_filter_test.js b/test/jasmine/tests/transform_filter_test.js index 25aa7684f95..67e0fdc7360 100644 --- a/test/jasmine/tests/transform_filter_test.js +++ b/test/jasmine/tests/transform_filter_test.js @@ -95,45 +95,6 @@ describe('one-to-one transforms:', function() { }, '- trace second'); }); - it('should pass correctly arguments to transform methods', function() { - var transformIn = { type: 'fake' }; - var transformOut = {}; - - var dataIn = [{ - transforms: [transformIn] - }]; - - var layout = {}; - - function assertSupplyDefaultsArgs(_transformIn, traceOut, _layout) { - expect(_transformIn).toBe(transformIn); - expect(_layout).toBe(layout); - - return transformOut; - } - - function assertTransformArgs(dataOut, opts) { - expect(dataOut[0]._input).toBe(dataIn[0]); - expect(opts.transform).toBe(transformOut); - expect(opts.fullTrace._input).toBe(dataIn[0]); - expect(opts.layout).toBe(layout); - - return dataOut; - } - - var fakeTransformModule = { - moduleType: 'transform', - name: 'fake', - attributes: {}, - supplyDefaults: assertSupplyDefaultsArgs, - transform: assertTransformArgs - }; - - Plotly.register(fakeTransformModule); - Plots.supplyDataDefaults(dataIn, [], layout); - delete Plots.transformsRegistry.fake; - }); - it('supplyDataDefaults should apply the transform while', function() { var dataIn = [{ x: [-2, -2, 1, 2, 3], diff --git a/test/jasmine/tests/transform_multi_test.js b/test/jasmine/tests/transform_multi_test.js index 0b63c2c8e5e..fa341b9d084 100644 --- a/test/jasmine/tests/transform_multi_test.js +++ b/test/jasmine/tests/transform_multi_test.js @@ -1,4 +1,5 @@ var Plotly = require('@lib/index'); +var Plots = require('@src/plots/plots'); var Lib = require('@src/lib'); var createGraphDiv = require('../assets/create_graph_div'); @@ -7,6 +8,50 @@ var assertDims = require('../assets/assert_dims'); var assertStyle = require('../assets/assert_style'); +describe('user-defined transforms:', function() { + 'use strict'; + + it('should pass correctly arguments to transform methods', function() { + var transformIn = { type: 'fake' }; + var transformOut = {}; + + var dataIn = [{ + transforms: [transformIn] + }]; + + var layout = {}; + + function assertSupplyDefaultsArgs(_transformIn, traceOut, _layout) { + expect(_transformIn).toBe(transformIn); + expect(_layout).toBe(layout); + + return transformOut; + } + + function assertTransformArgs(dataOut, opts) { + expect(dataOut[0]._input).toBe(dataIn[0]); + expect(opts.transform).toBe(transformOut); + expect(opts.fullTrace._input).toBe(dataIn[0]); + expect(opts.layout).toBe(layout); + + return dataOut; + } + + var fakeTransformModule = { + moduleType: 'transform', + name: 'fake', + attributes: {}, + supplyDefaults: assertSupplyDefaultsArgs, + transform: assertTransformArgs + }; + + Plotly.register(fakeTransformModule); + Plots.supplyDataDefaults(dataIn, [], layout); + delete Plots.transformsRegistry.fake; + }); + +}); + describe('multiple transforms:', function() { 'use strict'; From 64dc72af313a993b0ebd0c94839fc7d880af3518 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 28 Sep 2016 16:59:55 -0400 Subject: [PATCH 06/26] transforms: make findArrayAttributes a lib function - instead of calling it for every transform trace - call findArrayAttributes in transform function - make findArrayAttributes 'aware' of array attribute in trace so that it lists only the array attributes present in given trace --- src/lib/coerce.js | 50 +++++++++++++++++++++++++++++++++++++++ src/lib/index.js | 1 + src/plots/plots.js | 36 ---------------------------- src/transforms/groupby.js | 12 ++++------ 4 files changed, 56 insertions(+), 43 deletions(-) diff --git a/src/lib/coerce.js b/src/lib/coerce.js index b974f323016..d0f8565847f 100644 --- a/src/lib/coerce.js +++ b/src/lib/coerce.js @@ -409,3 +409,53 @@ exports.crawl = function(attrs, callback, specifiedLevel) { if(isPlainObject(attr)) exports.crawl(attr, callback, level + 1); }); }; + +/** + * Find all data array attributes in a given trace object - including + * `arrayOk` attributes. + * + * @param {object} trace + * full trace object that contains a reference to `_module.attributes` + * + * @return {array} arrayAttributes + * list of array attributes for the given trace + */ +exports.findArrayAttributes = function(trace) { + var arrayAttributes = [], + stack = []; + + /** + * A closure that gathers attribute paths into its enclosed arraySplitAttributes + * Attribute paths are collected iff their leaf node is a splittable attribute + * + * @callback callback + * @param {object} attr an attribute + * @param {String} attrName name string + * @param {object[]} attrs all the attributes + * @param {Number} level the recursion level, 0 at the root + * + * @closureVariable {String[][]} arrayAttributes the set of gathered attributes + * Example of filled closure variable (expected to be initialized to []): + * [["marker","size"],["marker","line","width"],["marker","line","color"]] + */ + function callback(attr, attrName, attrs, level) { + stack = stack.slice(0, level).concat([attrName]); + + var splittableAttr = attr.valType === 'data_array' || attr.arrayOk === true; + if(!splittableAttr) return; + + var astr = toAttrString(stack); + var val = nestedProperty(trace, astr).get(); + if(!Array.isArray(val)) return; + + arrayAttributes.push(astr); + } + + function toAttrString(stack) { + return stack.join('.'); + } + + exports.crawl(trace._module.attributes, callback); + + return arrayAttributes; +}; diff --git a/src/lib/index.js b/src/lib/index.js index 2cbe9af07c3..3518fca5d31 100644 --- a/src/lib/index.js +++ b/src/lib/index.js @@ -25,6 +25,7 @@ lib.coerceFont = coerceModule.coerceFont; lib.validate = coerceModule.validate; lib.isValObject = coerceModule.isValObject; lib.crawl = coerceModule.crawl; +lib.findArrayAttributes = coerceModule.findArrayAttributes; lib.IS_SUBPLOT_OBJ = coerceModule.IS_SUBPLOT_OBJ; lib.IS_LINKED_TO_ARRAY = coerceModule.IS_LINKED_TO_ARRAY; lib.DEPRECATED = coerceModule.DEPRECATED; diff --git a/src/plots/plots.js b/src/plots/plots.js index 195981f6185..4ccf4323597 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -786,41 +786,6 @@ function applyTransforms(fullTrace, fullData, layout) { var container = fullTrace.transforms, dataOut = [fullTrace]; - var attributeSets = dataOut.map(function(trace) { - - var arraySplitAttributes = []; - - var stack = []; - - /** - * A closure that gathers attribute paths into its enclosed arraySplitAttributes - * Attribute paths are collected iff their leaf node is a splittable attribute - * @callback callback - * @param {object} attr an attribute - * @param {String} attrName name string - * @param {object[]} attrs all the attributes - * @param {Number} level the recursion level, 0 at the root - * @closureVariable {String[][]} arraySplitAttributes the set of gathered attributes - * Example of filled closure variable (expected to be initialized to []): - * [["marker","size"],["marker","line","width"],["marker","line","color"]] - */ - function callback(attr, attrName, attrs, level) { - - stack = stack.slice(0, level).concat([attrName]); - - var splittableAttr = attr.valType === 'data_array' || attr.arrayOk === true; - if(splittableAttr) { - arraySplitAttributes.push(stack.slice()); - } - } - - Lib.crawl(trace._module.attributes, callback); - - return arraySplitAttributes.map(function(path) { - return path.join('.'); - }); - }); - for(var i = 0; i < container.length; i++) { var transform = container[i], type = transform.type, @@ -831,7 +796,6 @@ function applyTransforms(fullTrace, fullData, layout) { transform: transform, fullTrace: fullTrace, fullData: fullData, - attributeSets: attributeSets, layout: layout, transformIndex: i }); diff --git a/src/transforms/groupby.js b/src/transforms/groupby.js index 7838b6e76fc..de146079842 100644 --- a/src/transforms/groupby.js +++ b/src/transforms/groupby.js @@ -103,8 +103,7 @@ function pasteArray(newTrace, trace, j, a) { ); } -function transformOne(trace, state, attributeSet) { - +function transformOne(trace, state) { var opts = state.transform; var groups = trace.transforms[state.transformIndex].groups; @@ -119,10 +118,9 @@ function transformOne(trace, state, attributeSet) { var newData = new Array(groupNames.length); var len = groups.length; - var style = opts.style || {}; + var arrayAttrs = Lib.findArrayAttributes(trace); - var arrayAttributes = attributeSet - .filter(function(array) {return Array.isArray(Lib.nestedProperty(trace, array).get());}); + var style = opts.style || {}; for(var i = 0; i < groupNames.length; i++) { var groupName = groupNames[i]; @@ -131,12 +129,12 @@ function transformOne(trace, state, attributeSet) { // maybe we could abstract this out var newTrace = newData[i] = Lib.extendDeep({}, trace); - arrayAttributes.forEach(initializeArray.bind(null, newTrace)); + arrayAttrs.forEach(initializeArray.bind(null, newTrace)); for(var j = 0; j < len; j++) { if(groups[j] !== groupName) continue; - arrayAttributes.forEach(pasteArray.bind(0, newTrace, trace, j)); + arrayAttrs.forEach(pasteArray.bind(0, newTrace, trace, j)); } newTrace.name = groupName; From 123823608f6ad472a05e957d6fbe06024cfdee4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 28 Sep 2016 17:35:35 -0400 Subject: [PATCH 07/26] transforms: add 'calcTransform' handler in doCalcdata - to allow calcTransform methods where fullTrace / or calcdata items can be modified during the calc step. - this allows writing transforms that are applied after much of the 'computed' fields are filled e.g. axis (auto) 'type' - calcTransforms can only manipulate trace object 1-to-1 e.g. grouping operation should be done at the defaults step as previously. --- src/plots/plots.js | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/src/plots/plots.js b/src/plots/plots.js index 4ccf4323597..26f442f436c 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -1581,7 +1581,7 @@ plots.doCalcdata = function(gd, traces) { var axList = Plotly.Axes.list(gd), fullData = gd._fullData, fullLayout = gd._fullLayout, - i; + i, j; // XXX: Is this correct? Needs a closer look so that *some* traces can be recomputed without // *all* needing doCalcdata: @@ -1619,7 +1619,6 @@ plots.doCalcdata = function(gd, traces) { } var trace = fullData[i], - _module = trace._module, cd = []; // If traces were specified and this trace was not included, then transfer it over from @@ -1629,8 +1628,30 @@ plots.doCalcdata = function(gd, traces) { continue; } - if(_module && trace.visible === true) { - if(_module.calc) cd = _module.calc(gd, trace); + var _module; + if(trace.visible === true) { + + // call calcTransform method if any + if(trace.transforms) { + + // we need one round of of trace module calc before + // the calc transform to 'fill in' the categories list + // used for example in the data-to-coordinate method + _module = trace._module; + if(_module && _module.calc) _module.calc(gd, trace); + + for(j = 0; j < trace.transforms.length; j++) { + var transform = trace.transforms[j]; + + _module = transformsRegistry[transform.type]; + if(_module && _module.calcTransform) { + _module.calcTransform(gd, trace, transform); + } + } + } + + _module = trace._module; + if(_module && _module.calc) cd = _module.calc(gd, trace); } // make sure there is a first point From 0e1e9a79ab3f87fb08ce8737c36bc6ce6d1964a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 28 Sep 2016 17:37:42 -0400 Subject: [PATCH 08/26] transforms: filter take II - rewrite transform as a calcTransform - add 'active' (like in groupby) - add 'strictinterval' (boolean) to determine e.g. `>` vs `>=` - makes filter works with numerical, categorical and dates!!! --- src/transforms/filter.js | 342 +++++++++++++++++++++++---------------- 1 file changed, 205 insertions(+), 137 deletions(-) diff --git a/src/transforms/filter.js b/src/transforms/filter.js index d54e7358c1a..ace01d1069b 100644 --- a/src/transforms/filter.js +++ b/src/transforms/filter.js @@ -8,44 +8,65 @@ 'use strict'; -var isNumeric = require('fast-isnumeric'); - var Lib = require('../lib'); +var axisIds = require('../plots/cartesian/axis_ids'); exports.moduleType = 'transform'; exports.name = 'filter'; exports.attributes = { + active: { + valType: 'boolean', + dflt: true, + description: [ + 'Toggles whether or not the filter is active.' + ].join(' ') + }, + filtersrc: { + valType: 'enumerated', + values: ['x', 'y', 'z', 'ids'], + dflt: 'x', + description: [ + 'Sets the variable which the filter will be applied.' + ].join(' ') + }, operation: { valType: 'enumerated', values: ['=', '<', '>', 'within', 'notwithin', 'in', 'notin'], - dflt: '=' + dflt: '=', + description: [ + 'Sets the filter operation.' + ].join(' ') }, value: { valType: 'any', - dflt: 0 + dflt: 0, + description: [ + 'Sets the value or values by which to filter by.', + 'Values are expected to be in the same type as the data linked', + 'to *filtersrc*.', + 'When `operation` is set to *within* and *notwithin*', + '*value* is expected to be 2-item array where the first item', + 'is the lower bound and the second item is the upper bound.', + 'When `operation`, is set to *in*, *notin* ' + ].join(' ') }, - filtersrc: { - valType: 'enumerated', - values: ['x', 'y', 'ids'], - dflt: 'x' + strictinterval: { + valType: 'boolean', + dflt: true, + arrayOk: true, + description: [ + 'Determines whether or not the filter operation includes data item value,', + 'equal to *value*.', + 'Has only an effect for `operation` *>*, *<*, *within* and *notwithin*', + 'When `operation` is set to *within* and *notwithin*,', + '`strictinterval` is expected to be a 2-item array where the first (second)', + 'item determines strictness for the lower (second) bound.' + ].join(' ') } }; -/** - * Supply transform attributes defaults - * - * @param {object} transformIn - * object linked to trace.transforms[i] with 'type' set to exports.name - * @param {object} fullData - * the plot's full data - * @param {object} layout - * the plot's (not-so-full) layout - * - * @return {object} transformOut - * copy of transformIn that contains attribute defaults - */ exports.supplyDefaults = function(transformIn) { var transformOut = {}; @@ -53,74 +74,59 @@ exports.supplyDefaults = function(transformIn) { return Lib.coerce(transformIn, transformOut, exports.attributes, attr, dflt); } - coerce('operation'); - coerce('value'); - coerce('filtersrc'); - - // numeric values as character should be converted to numeric - if(Array.isArray(transformOut.value)) { - transformOut.value = transformOut.value.map(function(v) { - if(isNumeric(v)) v = +v; - return v; - }); - } else { - if(isNumeric(transformOut.value)) transformOut.value = +transformOut.value; - } + var active = coerce('active'); - // or some more complex logic using fullData and layout + if(active) { + var operation = coerce('operation'); + + coerce('value'); + coerce('filtersrc'); + + if(['=', 'in', 'notin'].indexOf(operation) === -1) { + coerce('strictinterval'); + } + } return transformOut; }; -/** - * Apply transform !!! - * - * @param {array} data - * array of transformed traces (is [fullTrace] upon first transform) - * - * @param {object} state - * state object which includes: - * - transform {object} full transform attributes - * - fullTrace {object} full trace object which is being transformed - * - fullData {array} full pre-transform(s) data array - * - layout {object} the plot's (not-so-full) layout - * - * @return {object} newData - * array of transformed traces - */ -exports.transform = function(data, state) { - - // one-to-one case - - var newData = data.map(function(trace) { - return transformOne(trace, state); - }); - - return newData; +exports.transform = function(data) { + return data; }; -function transformOne(trace, state) { - var newTrace = Lib.extendDeep({}, trace); +exports.calcTransform = function(gd, trace, opts) { + var filtersrc = opts.filtersrc; - var opts = state.transform; - var src = opts.filtersrc; - var filterFunc = getFilterFunc(opts); - var len = trace[src].length; - var arrayAttrs = findArrayAttributes(trace); + if(!opts.active || !trace[filtersrc]) return; - arrayAttrs.forEach(function(attr) { - Lib.nestedProperty(newTrace, attr).set([]); - }); + var dataToCoord = getDataToCoordFunc(gd, filtersrc), + filterFunc = getFilterFunc(opts, dataToCoord); + + var filterArr = trace[filtersrc], + len = filterArr.length; + + var arrayAttrs = Lib.findArrayAttributes(trace), + originalArrays = {}; + + // copy all original array attribute values, + // and clear arrays in trace + for(var k = 0; k < arrayAttrs.length; k++) { + var attr = arrayAttrs[k], + np = Lib.nestedProperty(trace, attr); + + originalArrays[attr] = Lib.extendDeep([], np.get()); + np.set([]); + } function fill(attr, i) { - var arr = Lib.nestedProperty(trace, attr).get(); - var newArr = Lib.nestedProperty(newTrace, attr).get(); + var oldArr = originalArrays[attr], + newArr = Lib.nestedProperty(trace, attr).get(); - newArr.push(arr[i]); + newArr.push(oldArr[i]); } for(var i = 0; i < len; i++) { - var v = trace[src][i]; + var v = filterArr[i]; if(!filterFunc(v)) continue; @@ -128,81 +134,143 @@ function transformOne(trace, state) { fill(arrayAttrs[j], i); } } +}; + +function getDataToCoordFunc(gd, filtersrc) { + var ax = axisIds.getFromId(gd, filtersrc); - return newTrace; + // if 'filtersrc' has corresponding axis + // -> use setConvert method + if(ax) return ax.d2c; + + // special case for 'ids' + // -> cast to String + if(filtersrc === 'ids') return function(v) { return String(v); }; + + // otherwise -> case to number + return function(v) { return +(v); }; } -function getFilterFunc(opts) { - var value = opts.value; - // if value is not array then coerce to - // an array of [value,value] so the - // filter function will work - // but perhaps should just error out - var valueArr = []; - if(!Array.isArray(value)) { - valueArr = [value, value]; - } else { - valueArr = value; +function getFilterFunc(opts, d2c) { + var operation = opts.operation, + value = opts.value, + hasArrayValue = Array.isArray(value), + strict = opts.strictinterval, + hasArrayStrict = Array.isArray(strict); + + function isOperationIn(array) { + return array.indexOf(operation) !== -1; + } + + var coercedValue, coercedStrict; + + if(isOperationIn(['=', '<', '>'])) { + coercedValue = hasArrayValue ? d2c(value[0]) : d2c(value); + + if(isOperationIn(['<', '>'])) { + coercedStrict = hasArrayStrict ? strict[0] : strict; + } + } + else if(isOperationIn(['within', 'notwithin'])) { + coercedValue = hasArrayValue ? + [d2c(value[0]), d2c(value[1])] : + [d2c(value), d2c(value)]; + + coercedStrict = hasArrayStrict ? + [strict[0], strict[1]] : + [strict, strict]; } + else if(isOperationIn(['in', 'notin'])) { + coercedValue = hasArrayValue ? value.map(d2c) : [d2c(value)]; + } + + switch(operation) { - switch(opts.operation) { case '=': - return function(v) { return v === value; }; + return function(v) { return d2c(v) === coercedValue; }; + case '<': - return function(v) { return v < value; }; + if(coercedStrict) { + return function(v) { return d2c(v) < coercedValue; }; + } + else { + return function(v) { return d2c(v) <= coercedValue; }; + } + case '>': - return function(v) { return v > value; }; - case 'within': - return function(v) { - // if character then ignore with no side effect - function notDateNumber(d) { - return !(isNumeric(d) || Lib.isDateTime(d)); - } - if(valueArr.some(notDateNumber)) { - return true; - } - - // keep the = ? - return v >= Math.min.apply(null, valueArr) && - v <= Math.max.apply(null, valueArr); - }; - case 'notwithin': - return function(v) { - // keep the = ? - return !(v >= Math.min.apply(null, valueArr) && - v <= Math.max.apply(null, valueArr)); - }; - case 'in': - return function(v) { return valueArr.indexOf(v) >= 0; }; - case 'notin': - return function(v) { return valueArr.indexOf(v) === -1; }; - } -} + if(coercedStrict) { + return function(v) { return d2c(v) > coercedValue; }; + } + else { + return function(v) { return d2c(v) >= coercedValue; }; + } -function findArrayAttributes(obj, root) { - root = root || ''; + case 'within': - var list = []; + if(coercedStrict[0] && coercedStrict[1]) { + return function(v) { + var cv = d2c(v); + return cv > coercedValue[0] && cv < coercedValue[1]; + }; + } + else if(coercedStrict[0] && !coercedStrict[1]) { + return function(v) { + var cv = d2c(v); + return cv > coercedValue[0] && cv <= coercedValue[1]; + }; + } + else if(!coercedStrict[0] && coercedStrict[1]) { + return function(v) { + var cv = d2c(v); + return cv >= coercedValue[0] && cv < coercedValue[1]; + }; + } + else if(!coercedStrict[0] && !coercedStrict[1]) { + return function(v) { + var cv = d2c(v); + return cv >= coercedValue[0] && cv <= coercedValue[1]; + }; + } + + break; - Object.keys(obj).forEach(function(k) { - var val = obj[k]; + case 'notwithin': - if(k.charAt(0) === '_') return; + if(coercedStrict[0] && coercedStrict[1]) { + return function(v) { + var cv = d2c(v); + return cv < coercedValue[0] || cv > coercedValue[1]; + }; + } + else if(coercedStrict[0] && !coercedStrict[1]) { + return function(v) { + var cv = d2c(v); + return cv < coercedValue[0] || cv >= coercedValue[1]; + }; + } + else if(!coercedStrict[0] && coercedStrict[1]) { + return function(v) { + var cv = d2c(v); + return cv <= coercedValue[0] || cv > coercedValue[1]; + }; + } + else if(!coercedStrict[0] && !coercedStrict[1]) { + return function(v) { + var cv = d2c(v); + return cv <= coercedValue[0] || cv >= coercedValue[1]; + }; + } + + break; - if(k === 'transforms') { - val.forEach(function(item, i) { - list = list.concat( - findArrayAttributes(item, root + k + '[' + i + ']' + '.') - ); - }); - } - else if(Lib.isPlainObject(val)) { - list = list.concat(findArrayAttributes(val, root + k + '.')); - } - else if(Array.isArray(val)) { - list.push(root + k); - } - }); + case 'in': + return function(v) { + return coercedValue.indexOf(d2c(v)) !== -1; + }; - return list; + case 'notin': + return function(v) { + return coercedValue.indexOf(d2c(v)) === -1; + }; + } } From f722c5d2b9368644f68340f827649eda82df1b47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 28 Sep 2016 17:38:24 -0400 Subject: [PATCH 09/26] transforms: fill in groupby descriptions --- src/transforms/groupby.js | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/src/transforms/groupby.js b/src/transforms/groupby.js index de146079842..62142b42d15 100644 --- a/src/transforms/groupby.js +++ b/src/transforms/groupby.js @@ -17,15 +17,31 @@ exports.name = 'groupby'; exports.attributes = { active: { valType: 'boolean', - dflt: true + dflt: true, + description: [ + 'Toggles whether or not the filter is active.' + ].join(' ') }, groups: { valType: 'data_array', - dflt: [] + dflt: [], + description: [ + 'Sets the groups in which the trace data will be split.', + 'For example, with `x` set to *[1, 2, 3, 4]* and', + '`groups` set to *[\'a\', \'b\', \'a\', \'b\']*,', + 'the groupby transform with split in one trace', + 'with `x` [1, 3] and one trace with `x` [2, 4].' + ].join(' ') }, style: { valType: 'any', - dflt: {} + dflt: {}, + description: [ + 'Sets each group style.', + 'For example, with `groups` set to *[\'a\', \'b\', \'a\', \'b\']*', + 'and `style` set to *{ a: { marker: { color: \'red\' } }}', + 'marker points in group *\'a\'* will be drawn in red.' + ].join(' ') } }; From 037fa5cb906ab60db2f3338fc77f61e7c4feb63d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 28 Sep 2016 17:39:19 -0400 Subject: [PATCH 10/26] transforms: perf in groupby - extend original traces with extendDeepNoArrays (no need to copy arrays - they are filled during the grouping operation) - swap .forEach for good old for loop --- src/transforms/groupby.js | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/src/transforms/groupby.js b/src/transforms/groupby.js index 62142b42d15..2af72d50175 100644 --- a/src/transforms/groupby.js +++ b/src/transforms/groupby.js @@ -72,8 +72,6 @@ exports.supplyDefaults = function(transformIn) { coerce('groups'); coerce('style'); - // or some more complex logic using fullData and layout - return transformOut; }; @@ -94,15 +92,11 @@ exports.supplyDefaults = function(transformIn) { * array of transformed traces */ exports.transform = function(data, state) { - - // one-to-many case - var newData = []; - data.forEach(function(trace, i) { - - newData = newData.concat(transformOne(trace, state, state.attributeSets[i])); - }); + for(var i = 0; i < data.length; i++) { + newData = newData.concat(transformOne(data[i], state)); + } return newData; }; @@ -141,9 +135,7 @@ function transformOne(trace, state) { for(var i = 0; i < groupNames.length; i++) { var groupName = groupNames[i]; - // TODO is this the best pattern ??? - // maybe we could abstract this out - var newTrace = newData[i] = Lib.extendDeep({}, trace); + var newTrace = newData[i] = Lib.extendDeepNoArrays({}, trace); arrayAttrs.forEach(initializeArray.bind(null, newTrace)); @@ -155,9 +147,9 @@ function transformOne(trace, state) { newTrace.name = groupName; - // there's no need to coerce style[groupName] here + // there's no need to coerce style[groupName] here // as another round of supplyDefaults is done on the transformed traces - newTrace = Lib.extendDeep(newTrace, style[groupName] || {}); + newTrace = Lib.extendDeepNoArrays(newTrace, style[groupName] || {}); } return newData; From 01f859573d3878e8784941191d9efcf868d0fd5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 28 Sep 2016 17:39:34 -0400 Subject: [PATCH 11/26] lint: some line spacing in plots.js --- src/plots/plots.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/plots/plots.js b/src/plots/plots.js index 26f442f436c..66391c6c299 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -16,7 +16,9 @@ var Plotly = require('../plotly'); var Registry = require('../registry'); var Lib = require('../lib'); var Color = require('../components/color'); + var plots = module.exports = {}; + var animationAttrs = require('./animation_attributes'); var frameAttrs = require('./frame_attributes'); @@ -788,8 +790,7 @@ function applyTransforms(fullTrace, fullData, layout) { for(var i = 0; i < container.length; i++) { var transform = container[i], - type = transform.type, - _module = transformsRegistry[type]; + _module = transformsRegistry[transform.type]; if(_module) { dataOut = _module.transform(dataOut, { From 8a29a8904a64dbf0a48d489ce73984395b76cc46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 28 Sep 2016 17:46:13 -0400 Subject: [PATCH 12/26] test: one big nasty commit untangling / adding transforms tests - move some more general cases our of filter_test -> multi_test - add some filter default cases (around 'active' and 'strictinterval') - add a bunch of calcTransform test (numeric, categorical and dates are now covered!) --- test/jasmine/tests/transform_filter_test.js | 871 +++++++++----------- test/jasmine/tests/transform_multi_test.js | 142 +++- 2 files changed, 514 insertions(+), 499 deletions(-) diff --git a/test/jasmine/tests/transform_filter_test.js b/test/jasmine/tests/transform_filter_test.js index 67e0fdc7360..ca766a73c0e 100644 --- a/test/jasmine/tests/transform_filter_test.js +++ b/test/jasmine/tests/transform_filter_test.js @@ -8,610 +8,474 @@ var assertDims = require('../assets/assert_dims'); var assertStyle = require('../assets/assert_style'); -describe('one-to-one transforms:', function() { - 'use strict'; - - var mockData0 = [{ - x: [-2, -1, -2, 0, 1, 2, 3], - y: [1, 2, 3, 1, 2, 3, 1], - transforms: [{ - type: 'filter', - operation: '>' - }] - }]; - - var mockData1 = [Lib.extendDeep({}, mockData0[0]), { - x: [20, 11, 12, 0, 1, 2, 3], - y: [1, 2, 3, 2, 5, 2, 0], - transforms: [{ - type: 'filter', - operation: '<', - value: 10 - }] - }]; +describe('filter transforms defaults:', function() { var traceIn, traceOut; - afterEach(destroyGraphDiv); - - it('supplyTraceDefaults should supply the transform defaults', function() { + it('supplyTraceDefaults should coerce all attributes', function() { traceIn = { - y: [2, 1, 2], - transforms: [{ type: 'filter' }] + x: [1, 2, 3], + transforms: [{ + type: 'filter', + value: 0 + }] }; traceOut = Plots.supplyTraceDefaults(traceIn, 0, {}); expect(traceOut.transforms).toEqual([{ type: 'filter', + active: true, operation: '=', value: 0, filtersrc: 'x' }]); }); - it('supplyTraceDefaults should not bail if transform module is not found', function() { + it('supplyTraceDefaults should not coerce attributes if active: false', function() { traceIn = { - y: [2, 1, 2], - transforms: [{ type: 'invalid' }] - }; - - traceOut = Plots.supplyTraceDefaults(traceIn, 0, {}); - - expect(traceOut.y).toBe(traceIn.y); - }); - - it('supplyTraceDefaults should honored global transforms', function() { - traceIn = { - y: [2, 1, 2], + x: [1, 2, 3], transforms: [{ + active: false, type: 'filter', - operation: '>', - value: '0', - filtersrc: 'x' - }] - }; - - var layout = { - _globalTransforms: [{ - type: 'filter' + value: 0 }] }; - traceOut = Plots.supplyTraceDefaults(traceIn, 0, layout); - - expect(traceOut.transforms[0]).toEqual({ - type: 'filter', - operation: '=', - value: 0, - filtersrc: 'x' - }, '- global first'); + traceOut = Plots.supplyTraceDefaults(traceIn, 0, {}); - expect(traceOut.transforms[1]).toEqual({ + expect(traceOut.transforms).toEqual([{ type: 'filter', - operation: '>', - value: 0, - filtersrc: 'x' - }, '- trace second'); + active: false, + }]); }); - it('supplyDataDefaults should apply the transform while', function() { - var dataIn = [{ - x: [-2, -2, 1, 2, 3], - y: [1, 2, 2, 3, 1] - }, { - x: [-2, -1, -2, 0, 1, 2, 3], - y: [1, 2, 3, 1, 2, 3, 1], + it('supplyTraceDefaults should not coerce \'strictinterval\' when it has no meaning', function() { + traceIn = { + x: [1, 2, 3], transforms: [{ type: 'filter', - operation: '>', - value: '0', - filtersrc: 'x' + value: 0, + operation: '=', }] - }]; - - var dataOut = []; - Plots.supplyDataDefaults(dataIn, dataOut, {}, []); - - var msg; - - msg = 'does not mutate user data'; - expect(dataIn[1].x).toEqual([-2, -1, -2, 0, 1, 2, 3], msg); - expect(dataIn[1].y).toEqual([1, 2, 3, 1, 2, 3, 1], msg); - expect(dataIn[1].transforms).toEqual([{ - type: 'filter', - operation: '>', - value: '0', - filtersrc: 'x' - }], msg); - - msg = 'applies transform'; - expect(dataOut[1].x).toEqual([1, 2, 3], msg); - expect(dataOut[1].y).toEqual([2, 3, 1], msg); - - msg = 'supplying the transform defaults'; - expect(dataOut[1].transforms[0]).toEqual({ - type: 'filter', - operation: '>', - value: 0, - filtersrc: 'x' - }, msg); - - msg = 'keeping refs to user data'; - expect(dataOut[1]._input.x).toEqual([-2, -1, -2, 0, 1, 2, 3], msg); - expect(dataOut[1]._input.y).toEqual([1, 2, 3, 1, 2, 3, 1], msg); - expect(dataOut[1]._input.transforms).toEqual([{ - type: 'filter', - operation: '>', - value: '0', - filtersrc: 'x' - }], msg); - - msg = 'keeping refs to full transforms array'; - expect(dataOut[1]._fullInput.transforms).toEqual([{ - type: 'filter', - operation: '>', - value: 0, - filtersrc: 'x' - }], msg); - - msg = 'setting index w.r.t user data'; - expect(dataOut[0].index).toEqual(0, msg); - expect(dataOut[1].index).toEqual(1, msg); - - msg = 'setting _expandedIndex w.r.t full data'; - expect(dataOut[0]._expandedIndex).toEqual(0, msg); - expect(dataOut[1]._expandedIndex).toEqual(1, msg); - }); - - it('Plotly.plot should plot the transform trace', function(done) { - var data = Lib.extendDeep([], mockData0); - - Plotly.plot(createGraphDiv(), data).then(function(gd) { - assertDims([3]); - - var uid = data[0].uid; - expect(gd._fullData[0].uid).toEqual(uid + '0'); - - done(); - }); - }); - - it('Plotly.restyle should work', function(done) { - var data = Lib.extendDeep([], mockData0); - data[0].marker = { color: 'red' }; - - var gd = createGraphDiv(); - var dims = [3]; - - var uid; - function assertUid(gd) { - expect(gd._fullData[0].uid) - .toEqual(uid + '0', 'should preserve uid on restyle'); - } - - Plotly.plot(gd, data).then(function() { - uid = gd.data[0].uid; - - expect(gd._fullData[0].marker.color).toEqual('red'); - assertUid(gd); - assertStyle(dims, ['rgb(255, 0, 0)'], [1]); - - return Plotly.restyle(gd, 'marker.color', 'blue'); - }).then(function() { - expect(gd._fullData[0].marker.color).toEqual('blue'); - assertUid(gd); - assertStyle(dims, ['rgb(0, 0, 255)'], [1]); - - return Plotly.restyle(gd, 'marker.color', 'red'); - }).then(function() { - expect(gd._fullData[0].marker.color).toEqual('red'); - assertUid(gd); - assertStyle(dims, ['rgb(255, 0, 0)'], [1]); - - return Plotly.restyle(gd, 'transforms[0].value', 2.5); - }).then(function() { - assertUid(gd); - assertStyle([1], ['rgb(255, 0, 0)'], [1]); - - done(); - }); - }); - - it('Plotly.extendTraces should work', function(done) { - var data = Lib.extendDeep([], mockData0); - - var gd = createGraphDiv(); - - Plotly.plot(gd, data).then(function() { - expect(gd.data[0].x.length).toEqual(7); - expect(gd._fullData[0].x.length).toEqual(3); - - assertDims([3]); - - return Plotly.extendTraces(gd, { - x: [ [-3, 4, 5] ], - y: [ [1, -2, 3] ] - }, [0]); - }).then(function() { - expect(gd.data[0].x.length).toEqual(10); - expect(gd._fullData[0].x.length).toEqual(5); - - assertDims([5]); - - done(); - }); - }); - - it('Plotly.deleteTraces should work', function(done) { - var data = Lib.extendDeep([], mockData1); - - var gd = createGraphDiv(); - - Plotly.plot(gd, data).then(function() { - assertDims([3, 4]); + }; - return Plotly.deleteTraces(gd, [1]); - }).then(function() { - assertDims([3]); + traceOut = Plots.supplyTraceDefaults(traceIn, 0, {}); + expect(traceOut.transforms[0].strictinterval).toBeUndefined(); - return Plotly.deleteTraces(gd, [0]); - }).then(function() { - assertDims([]); + Lib.extendDeep(traceIn, { transforms: [{operation: 'in'}]}); + traceOut = Plots.supplyTraceDefaults(traceIn, 0, {}); + expect(traceOut.transforms[0].strictinterval).toBeUndefined(); - done(); - }); + Lib.extendDeep(traceIn, { transforms: [{operation: 'notin'}]}); + traceOut = Plots.supplyTraceDefaults(traceIn, 0, {}); + expect(traceOut.transforms[0].strictinterval).toBeUndefined(); + Lib.extendDeep(traceIn, { transforms: [{operation: '>'}]}); + traceOut = Plots.supplyTraceDefaults(traceIn, 0, {}); + expect(traceOut.transforms[0].strictinterval).toEqual(true); }); +}); - it('toggling trace visibility should work', function(done) { - var data = Lib.extendDeep([], mockData1); - - var gd = createGraphDiv(); - - Plotly.plot(gd, data).then(function() { - assertDims([3, 4]); - - return Plotly.restyle(gd, 'visible', 'legendonly', [1]); - }).then(function() { - assertDims([3]); - - return Plotly.restyle(gd, 'visible', false, [0]); - }).then(function() { - assertDims([]); - - return Plotly.restyle(gd, 'visible', [true, true], [0, 1]); - }).then(function() { - assertDims([3, 4]); - - done(); - }); - }); +describe('filter transforms calc:', function() { + 'use strict'; + function calcDatatoTrace(calcTrace) { + return calcTrace[0].trace; + } - it('supplyTraceDefaults should supply the transform defaults', function() { - traceIn = { - y: [2, 1, 2], - transforms: [{ type: 'filter' }] + function _transform(data) { + var gd = { + data: data, + layout: {} }; - traceOut = Plots.supplyTraceDefaults(traceIn, 0, {}); + Plots.supplyDefaults(gd); + Plots.doCalcdata(gd); - expect(traceOut.transforms).toEqual([{ - type: 'filter', - operation: '=', - value: 0, - filtersrc: 'x' - }]); - }); + return gd.calcdata.map(calcDatatoTrace); + } - it('supplyTraceDefaults should accept numeric as character', function() { - traceIn = { - x: '1', + var base = { + x: [-2, -1, -2, 0, 1, 2, 3], + y: [1, 2, 3, 1, 2, 3, 1], + ids: ['n0', 'n1', 'n2', 'z', 'p1', 'p2', 'p3'], + marker: { + color: [0.1, 0.2, 0.3, 0.1, 0.2, 0.3, 0.4], + size: 20 + }, + transforms: [{ type: 'filter' }] + }; + + it('filters should skip if *filtersrc* isn\'t present in trace', function() { + var out = _transform([Lib.extendDeep({}, base, { transforms: [{ type: 'filter', - value: '0' + operation: '>', + value: 0, + filtersrc: 'z' }] - }; - - traceOut = Plots.supplyTraceDefaults(traceIn, 0, {}); + })]); - expect(traceOut.transforms).toEqual([{ - type: 'filter', - operation: '=', - value: 0, - filtersrc: 'x' - }]); + expect(out[0].x).toEqual(base.x); + expect(out[0].y).toEqual(base.y); + }); - // should also convert if array - traceIn = { - x: '1', + it('filters should skip if *active* is false', function() { + var out = _transform([Lib.extendDeep({}, base, { transforms: [{ type: 'filter', - value: ['0'] + active: false, + operation: '>', + value: 0, + filtersrc: 'x' }] - }; + })]); - traceOut = Plots.supplyTraceDefaults(traceIn, 0, {}); - - expect(traceOut.transforms).toEqual([{ - type: 'filter', - operation: '=', - value: [0], - filtersrc: 'x' - }]); + expect(out[0].x).toEqual(base.x); + expect(out[0].y).toEqual(base.y); }); - it('supplyTraceDefaults should accept numeric as character', function() { - traceIn = { - x: '1', + it('filters should chain as AND (case 1)', function() { + var out = _transform([Lib.extendDeep({}, base, { transforms: [{ type: 'filter', - value: '0' - }] - }; - - traceOut = Plots.supplyTraceDefaults(traceIn, 0, {}); - - expect(traceOut.transforms).toEqual([{ - type: 'filter', - operation: '=', - value: 0, - filtersrc: 'x' - }]); - - // should also convert if array - traceIn = { - x: '1', - transforms: [{ + operation: '>', + value: 0, + filtersrc: 'x' + }, { type: 'filter', - value: ['0'] + operation: '<', + value: 3, + filtersrc: 'x' }] - }; - - traceOut = Plots.supplyTraceDefaults(traceIn, 0, {}); + })]); - expect(traceOut.transforms).toEqual([{ - type: 'filter', - operation: '=', - value: [0], - filtersrc: 'x' - }]); + expect(out[0].x).toEqual([1, 2]); + expect(out[0].y).toEqual([2, 3]); }); - it('supplyDataDefaults should apply the transform', function() { - var dataIn = [{ - x: [-2, -1, -2, 0, 1, 2, 3], - y: [1, 2, 3, 1, 2, 3, 1], + it('filters should chain as AND (case 2)', function() { + var out = _transform([Lib.extendDeep({}, base, { transforms: [{ type: 'filter', operation: '>', value: 0, filtersrc: 'x' + }, { + type: 'filter', + active: false, + operation: '>', + value: 2, + filtersrc: 'y' + }, { + type: 'filter', + operation: '<', + value: 2, + filtersrc: 'y' }] - }]; + })]); - var dataOut = []; - Plots.supplyDataDefaults(dataIn, dataOut, {}, []); + expect(out[0].x).toEqual([3]); + expect(out[0].y).toEqual([1]); + }); - // does not mutate user data - expect(dataIn[0].x).toEqual([-2, -1, -2, 0, 1, 2, 3]); - expect(dataIn[0].y).toEqual([1, 2, 3, 1, 2, 3, 1]); - expect(dataIn[0].transforms).toEqual([{ - type: 'filter', - operation: '>', - value: 0, - filtersrc: 'x' - }]); + describe('filters should handle numeric values', function() { + var _base = Lib.extendDeep({}, base); - // applies transform - expect(dataOut[0].x).toEqual([1, 2, 3]); - expect(dataOut[0].y).toEqual([2, 3, 1]); + function _assert(out, x, y, markerColor) { + expect(out[0].x).toEqual(x, '- x coords'); + expect(out[0].y).toEqual(y, '- y coords'); + expect(out[0].marker.color).toEqual(markerColor, '- marker.color arrayOk'); + expect(out[0].marker.size).toEqual(20, '- marker.size style'); + } - // TODO what is the expected behavior ??? -// expect(dataOut[0].transforms).toEqual([]); + it('+ *within*', function() { + var out = _transform([Lib.extendDeep({}, _base, { + transforms: [{ + operation: 'within', + value: [-1, 1], + strictinterval: [false, false], + filtersrc: 'x' + }] + })]); + + _assert( + out, + [-1, 0, 1], + [2, 1, 2], + [0.2, 0.1, 0.2] + ); + }); - // keep ref to user data - expect(dataOut[0]._input.x).toEqual([-2, -1, -2, 0, 1, 2, 3]); - expect(dataOut[0]._input.y).toEqual([1, 2, 3, 1, 2, 3, 1]); - expect(dataOut[0]._input.transforms).toEqual([{ - type: 'filter', - operation: '>', - value: 0, - filtersrc: 'x' - }]); + it('+ *notwithin*', function() { + var out = _transform([Lib.extendDeep({}, _base, { + transforms: [{ + operation: 'notwithin', + value: [-1, 1], + filtersrc: 'x' + }] + })]); + + _assert(out, + [-2, -2, 2, 3], + [1, 3, 3, 1], + [0.1, 0.3, 0.3, 0.4] + ); + }); - // keep ref to full transforms array - expect(dataOut[0]._fullInput.transforms).toEqual([{ - type: 'filter', - operation: '>', - value: 0, - filtersrc: 'x' - }]); + it('+ *in*', function() { + var out = _transform([Lib.extendDeep({}, _base, { + transforms: [{ + operation: 'in', + value: [-2, 0], + filtersrc: 'x' + }] + })]); + + _assert(out, + [-2, -2, 0], + [1, 3, 1], + [0.1, 0.3, 0.1] + ); + }); - // set index w.r.t. fullData - expect(dataOut[0].index).toEqual(0); + it('+ *notin*', function() { + var out = _transform([Lib.extendDeep({}, _base, { + transforms: [{ + operation: 'notin', + value: [-2, 0], + filtersrc: 'x' + }] + })]); + + _assert(out, + [-1, 1, 2, 3], + [2, 2, 3, 1], + [0.2, 0.2, 0.3, 0.4] + ); + }); - // TODO do we really need this ??? - // set _index w.r.t. user data - expect(dataOut[0].index).toEqual(0); }); - it('filters should chain as AND', function() { - var dataIn = [{ - x: [-2, -1, -2, 0, 1, 2, 3], - y: [1, 2, 3, 1, 2, 3, 1], - transforms: [ - { - type: 'filter', - operation: '>', - value: 0, + describe('filters should handle categories', function() { + var _base = { + x: ['a', 'b', 'c', 'd'], + y: [1, 2, 3, 4], + marker: { + color: 'red', + size: ['0', '1', '2', '0'] + }, + transforms: [{ type: 'filter' }] + }; + + function _assert(out, x, y, markerSize) { + expect(out[0].x).toEqual(x, '- x coords'); + expect(out[0].y).toEqual(y, '- y coords'); + expect(out[0].marker.size).toEqual(markerSize, '- marker.size arrayOk'); + expect(out[0].marker.color).toEqual('red', '- marker.color style'); + } + + it('+ *within*', function() { + var out = _transform([Lib.extendDeep({}, _base, { + transforms: [{ + operation: 'within', + value: ['a', 'c'], filtersrc: 'x' - }, - { - type: 'filter', - operation: '<', - value: 3, + }] + })]); + + _assert(out, ['b'], [2], ['1']); + }); + + it('+ *notwithin*', function() { + var out = _transform([Lib.extendDeep({}, _base, { + transforms: [{ + operation: 'notwithin', + value: ['a', 'c'], filtersrc: 'x' - } - ] - }]; + }] + })]); - var dataOut = []; - Plots.supplyDataDefaults(dataIn, dataOut, {}, []); + _assert(out, ['d'], [4], ['0']); + }); - // applies transform - expect(dataOut[0].x).toEqual([1, 2]); - expect(dataOut[0].y).toEqual([2, 3]); - }); + it('filters should handle categories + *in*', function() { + var out = _transform([Lib.extendDeep({}, _base, { + transforms: [{ + operation: 'in', + value: ['b', 'd'], + filtersrc: 'x' + }] + })]); - it('filters should handle range numeric values within and notwithin', function() { - var dataIn = [{ - x: [-2, -1, -2, 0, 1, 2, 3], - y: [1, 2, 3, 1, 2, 3, 1], - transforms: [{ - type: 'filter', - operation: 'within', - value: [-1, 1], - filtersrc: 'x' - }] - }]; + _assert(out, ['b', 'd'], [2, 4], ['1', '0']); + }); - var dataOut = []; - Plots.supplyDataDefaults(dataIn, dataOut, {}, []); + it('filters should handle categories + *notin*', function() { + var out = _transform([Lib.extendDeep({}, _base, { + transforms: [{ + operation: 'notin', + value: ['b', 'd'], + filtersrc: 'x' + }] + })]); - // leave this section guarding against mutation - // for now but can probably eliminate later - // does not mutate user data - expect(dataIn[0].x).toEqual([-2, -1, -2, 0, 1, 2, 3]); - expect(dataIn[0].y).toEqual([1, 2, 3, 1, 2, 3, 1]); - expect(dataIn[0].transforms).toEqual([{ - type: 'filter', - operation: 'within', - value: [-1, 1], - filtersrc: 'x' - }]); + _assert(out, ['a', 'c'], [1, 3], ['0', '2']); + }); - // applies transform - expect(dataOut[0].x).toEqual([-1, 0, 1]); - expect(dataOut[0].y).toEqual([2, 1, 2]); }); - it('filters should ignore character values within and notwithin', function() { - var dataIn = [{ - x: ['a', 'b', 'c', 'd'], - y: [1, 2, 3, 4], - transforms: [{ - type: 'filter', - operation: 'within', - value: ['a', 'c'], - filtersrc: 'x' - }] - }]; + describe('filters should handle dates', function() { + var _base = { + x: ['2015-07-20', '2016-08-01', '2016-09-01', '2016-10-21', '2016-12-02'], + y: [1, 2, 3, 1, 5], + marker: { + line: { + color: [0.1, 0.2, 0.3, 0.1, 0.2], + width: 2.5 + } + }, + transforms: [{ type: 'filter' }] + }; - var dataOut = []; - Plots.supplyDataDefaults(dataIn, dataOut, {}, []); + function _assert(out, x, y, markerLineColor) { + expect(out[0].x).toEqual(x, '- x coords'); + expect(out[0].y).toEqual(y, '- y coords'); + expect(out[0].marker.line.color).toEqual(markerLineColor, '- marker.line.color arrayOk'); + expect(out[0].marker.line.width).toEqual(2.5, '- marker.line.width style'); + } - // leave this section guarding against mutation - // for now but can probably eliminate later - // does not mutate user data - expect(dataIn[0].x).toEqual(['a', 'b', 'c', 'd']); - expect(dataIn[0].y).toEqual([1, 2, 3, 4]); - expect(dataIn[0].transforms).toEqual([{ - type: 'filter', - operation: 'within', - value: ['a', 'c'], - filtersrc: 'x' - }]); + it('+ *=*', function() { + var out = _transform([Lib.extendDeep({}, _base, { + transforms: [{ + operation: '=', + value: ['2015-07-20'], + filtersrc: 'x' + }] + })]); - // applies transform - expect(dataOut[0].x).toEqual(['a', 'b', 'c', 'd']); - expect(dataOut[0].y).toEqual([1, 2, 3, 4]); - }); + _assert(out, ['2015-07-20'], [1], [0.1]); + }); - it('filters should handle numeric values in', function() { - var dataIn = [{ - x: [-2, -1, -2, 0, 1, 2, 3], - y: [1, 2, 3, 1, 2, 3, 1], - transforms: [{ - type: 'filter', - operation: 'in', - value: [-2, 0], - filtersrc: 'x' - }] - }]; + it('+ *<*', function() { + var out = _transform([Lib.extendDeep({}, _base, { + transforms: [{ + operation: '<', + value: '2016-01-01', + filtersrc: 'x' + }] + })]); - var dataOut = []; - Plots.supplyDataDefaults(dataIn, dataOut, {}, []); + _assert(out, ['2015-07-20'], [1], [0.1]); + }); - // applies transform - expect(dataOut[0].x).toEqual([-2, -2, 0]); - expect(dataOut[0].y).toEqual([1, 3, 1]); - }); + it('+ *>*', function() { + var out = _transform([Lib.extendDeep({}, _base, { + transforms: [{ + operation: '>', + value: '2016-08-01', + strictinterval: false, + filtersrc: 'x' + }] + })]); + + _assert(out, + ['2016-08-01', '2016-09-01', '2016-10-21', '2016-12-02'], + [2, 3, 1, 5], + [0.2, 0.3, 0.1, 0.2] + ); + }); - it('filters should handle numeric values in', function() { - var dataIn = [{ - x: [-2, -1, -2, 0, 1, 2, 3], - y: [1, 2, 3, 1, 2, 3, 1], - transforms: [{ - type: 'filter', - operation: 'notin', - value: [-2, 0], - filtersrc: 'x' - }] - }]; + it('+ *within*', function() { + var out = _transform([Lib.extendDeep({}, _base, { + transforms: [{ + operation: 'within', + value: ['2016-08-01', '2016-10-01'], + strictinterval: false, + filtersrc: 'x' + }] + })]); - var dataOut = []; - Plots.supplyDataDefaults(dataIn, dataOut, {}, []); + _assert(out, ['2016-08-01', '2016-09-01'], [2, 3], [0.2, 0.3]); + }); - // applies transform - expect(dataOut[0].x).toEqual([-1, 1, 2, 3]); - expect(dataOut[0].y).toEqual([2, 2, 3, 1]); - }); + it('+ *notwithin*', function() { + var out = _transform([Lib.extendDeep({}, _base, { + transforms: [{ + operation: 'notwithin', + value: ['2016-08-01', '2016-10-01'], + filtersrc: 'x' + }] + })]); + _assert(out, ['2015-07-20', '2016-10-21', '2016-12-02'], [1, 1, 5], [0.1, 0.1, 0.2]); + }); - it('filters should handle strings with in', function() { - var dataIn = [{ - x: ['y', 't', 'b', 'm', 'p', 'l', 'o'], - y: [1, 2, 3, 1, 5, 10, 20], - transforms: [{ - type: 'filter', - operation: 'in', - value: ['p', 'l', 'o'], - filtersrc: 'x' - }] - }]; + it('+ *in*', function() { + var out = _transform([Lib.extendDeep({}, _base, { + transforms: [{ + operation: 'in', + value: '2015-07-20', + filtersrc: 'x' + }] + })]); + _assert(out, ['2015-07-20'], [1], [0.1]); + }); - var dataOut = []; - Plots.supplyDataDefaults(dataIn, dataOut, {}, []); + it('+ *notin*', function() { + var out = _transform([Lib.extendDeep({}, _base, { + transforms: [{ + operation: 'notin', + value: ['2016-08-01', '2016-09-01', '2016-10-21', '2016-12-02'], + filtersrc: 'x' + }] + })]); + + _assert(out, ['2015-07-20'], [1], [0.1]); + }); - // applies transform - expect(dataOut[0].x).toEqual(['p', 'l', 'o']); - expect(dataOut[0].y).toEqual([5, 10, 20]); }); - it('filters should handle strings with in', function() { - var dataIn = [{ - x: ['y', 't', 'b', 'm', 'p', 'l', 'o'], - y: [1, 2, 3, 1, 5, 10, 20], + it('filters should handle ids', function() { + var out = _transform([Lib.extendDeep({}, base, { transforms: [{ - type: 'filter', - operation: 'notin', - value: ['p', 'l', 'o'], - filtersrc: 'x' + operation: 'in', + value: ['p1', 'p2', 'n1'], + filtersrc: 'ids' }] - }]; + })]); + expect(out[0].x).toEqual([-1, 1, 2]); + expect(out[0].y).toEqual([2, 2, 3]); + expect(out[0].ids).toEqual(['n1', 'p1', 'p2']); + }); +}); - var dataOut = []; - Plots.supplyDataDefaults(dataIn, dataOut, {}, []); +describe('filter transforms interactions', function() { + 'use strict'; - // applies transform - expect(dataOut[0].x).toEqual(['y', 't', 'b', 'm']); - expect(dataOut[0].y).toEqual([1, 2, 3, 1]); - }); + var mockData0 = [{ + x: [-2, -1, -2, 0, 1, 2, 3], + y: [1, 2, 3, 1, 2, 3, 1], + transforms: [{ + type: 'filter', + operation: '>' + }] + }]; + var mockData1 = [Lib.extendDeep({}, mockData0[0]), { + x: [20, 11, 12, 0, 1, 2, 3], + y: [1, 2, 3, 2, 5, 2, 0], + transforms: [{ + type: 'filter', + operation: '<', + value: 10 + }] + }]; + + afterEach(destroyGraphDiv); it('Plotly.plot should plot the transform trace', function(done) { var data = Lib.extendDeep([], mockData0); @@ -633,22 +497,34 @@ describe('one-to-one transforms:', function() { var gd = createGraphDiv(); var dims = [3]; + var uid; + function assertUid(gd) { + expect(gd._fullData[0].uid) + .toEqual(uid + '0', 'should preserve uid on restyle'); + } + Plotly.plot(gd, data).then(function() { + uid = gd.data[0].uid; + expect(gd._fullData[0].marker.color).toEqual('red'); + assertUid(gd); assertStyle(dims, ['rgb(255, 0, 0)'], [1]); return Plotly.restyle(gd, 'marker.color', 'blue'); }).then(function() { expect(gd._fullData[0].marker.color).toEqual('blue'); + assertUid(gd); assertStyle(dims, ['rgb(0, 0, 255)'], [1]); return Plotly.restyle(gd, 'marker.color', 'red'); }).then(function() { expect(gd._fullData[0].marker.color).toEqual('red'); + assertUid(gd); assertStyle(dims, ['rgb(255, 0, 0)'], [1]); return Plotly.restyle(gd, 'transforms[0].value', 2.5); }).then(function() { + assertUid(gd); assertStyle([1], ['rgb(255, 0, 0)'], [1]); done(); @@ -724,5 +600,4 @@ describe('one-to-one transforms:', function() { done(); }); }); - }); diff --git a/test/jasmine/tests/transform_multi_test.js b/test/jasmine/tests/transform_multi_test.js index fa341b9d084..fe5e7b7c734 100644 --- a/test/jasmine/tests/transform_multi_test.js +++ b/test/jasmine/tests/transform_multi_test.js @@ -8,6 +8,147 @@ var assertDims = require('../assets/assert_dims'); var assertStyle = require('../assets/assert_style'); +describe('general transforms:', function() { + 'use strict'; + + var traceIn, traceOut; + + it('supplyTraceDefaults should supply the transform defaults', function() { + traceIn = { + y: [2, 1, 2], + transforms: [{ type: 'filter' }] + }; + + traceOut = Plots.supplyTraceDefaults(traceIn, 0, {}); + + expect(traceOut.transforms).toEqual([{ + type: 'filter', + active: true, + operation: '=', + value: 0, + filtersrc: 'x' + }]); + }); + + it('supplyTraceDefaults should not bail if transform module is not found', function() { + traceIn = { + y: [2, 1, 2], + transforms: [{ type: 'invalid' }] + }; + + traceOut = Plots.supplyTraceDefaults(traceIn, 0, {}); + + expect(traceOut.y).toBe(traceIn.y); + }); + + it('supplyTraceDefaults should honored global transforms', function() { + traceIn = { + y: [2, 1, 2], + transforms: [{ + type: 'filter', + operation: '>', + value: 0, + filtersrc: 'x' + }] + }; + + var layout = { + _globalTransforms: [{ + type: 'filter' + }] + }; + + traceOut = Plots.supplyTraceDefaults(traceIn, 0, layout); + + expect(traceOut.transforms[0]).toEqual({ + type: 'filter', + active: true, + operation: '=', + value: 0, + filtersrc: 'x' + }, '- global first'); + + expect(traceOut.transforms[1]).toEqual({ + type: 'filter', + active: true, + operation: '>', + strictinterval: true, + value: 0, + filtersrc: 'x' + }, '- trace second'); + }); + + it('supplyDataDefaults should apply the transform while', function() { + var dataIn = [{ + x: [-2, -2, 1, 2, 3], + y: [1, 2, 2, 3, 1] + }, { + x: [-2, -1, -2, 0, 1, 2, 3], + y: [1, 2, 3, 1, 2, 3, 1], + transforms: [{ + type: 'filter', + operation: '>', + value: 0, + filtersrc: 'x' + }] + }]; + + var dataOut = []; + Plots.supplyDataDefaults(dataIn, dataOut, {}, []); + + var msg; + + msg = 'does not mutate user data'; + expect(dataIn[1].x).toEqual([-2, -1, -2, 0, 1, 2, 3], msg); + expect(dataIn[1].y).toEqual([1, 2, 3, 1, 2, 3, 1], msg); + expect(dataIn[1].transforms).toEqual([{ + type: 'filter', + operation: '>', + value: 0, + filtersrc: 'x' + }], msg); + + msg = 'supplying the transform defaults'; + expect(dataOut[1].transforms[0]).toEqual({ + type: 'filter', + active: true, + operation: '>', + value: 0, + strictinterval: true, + filtersrc: 'x' + }, msg); + + msg = 'keeping refs to user data'; + expect(dataOut[1]._input.x).toEqual([-2, -1, -2, 0, 1, 2, 3], msg); + expect(dataOut[1]._input.y).toEqual([1, 2, 3, 1, 2, 3, 1], msg); + expect(dataOut[1]._input.transforms).toEqual([{ + type: 'filter', + operation: '>', + value: 0, + filtersrc: 'x' + }], msg); + + msg = 'keeping refs to full transforms array'; + expect(dataOut[1]._fullInput.transforms).toEqual([{ + type: 'filter', + active: true, + operation: '>', + value: 0, + strictinterval: true, + filtersrc: 'x' + }], msg); + + msg = 'setting index w.r.t user data'; + expect(dataOut[0].index).toEqual(0, msg); + expect(dataOut[1].index).toEqual(1, msg); + + msg = 'setting _expandedIndex w.r.t full data'; + expect(dataOut[0]._expandedIndex).toEqual(0, msg); + expect(dataOut[1]._expandedIndex).toEqual(1, msg); + }); + +}); + describe('user-defined transforms:', function() { 'use strict'; @@ -432,5 +573,4 @@ describe('multiple traces with transforms:', function() { done(); }); }); - }); From ad5089eab207f7ca4db403dc56fd1120597ef408 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Thu, 29 Sep 2016 11:53:53 -0400 Subject: [PATCH 13/26] test: make sure all '#graph' nodes are purged after toimage tests --- test/jasmine/tests/toimage_test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/jasmine/tests/toimage_test.js b/test/jasmine/tests/toimage_test.js index 7bfab046784..26fc91f872b 100644 --- a/test/jasmine/tests/toimage_test.js +++ b/test/jasmine/tests/toimage_test.js @@ -22,6 +22,7 @@ describe('Plotly.toImage', function() { // make sure ALL graph divs are deleted, // even the ones generated by Plotly.toImage d3.selectAll('.js-plotly-plot').remove(); + d3.selectAll('#graph').remove(); }); it('should be attached to Plotly', function() { From 7a98f150c4089ec8e45591f6d857ebbdde7efbbb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Thu, 29 Sep 2016 11:57:59 -0400 Subject: [PATCH 14/26] test: fixup plotschema case for new filter attributes --- test/jasmine/tests/plotschema_test.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/jasmine/tests/plotschema_test.js b/test/jasmine/tests/plotschema_test.js index 013db8d2279..e16aa5e8b82 100644 --- a/test/jasmine/tests/plotschema_test.js +++ b/test/jasmine/tests/plotschema_test.js @@ -176,6 +176,8 @@ describe('plot schema', function() { var valObjects = plotSchema.transforms.filter.attributes, attrNames = Object.keys(valObjects); - expect(attrNames).toEqual(['operation', 'value', 'filtersrc']); + ['operation', 'value', 'filtersrc'].forEach(function(k) { + expect(attrNames).toContain(k); + }); }); }); From c8a13b8d715380f08fabff006c5a4f5a90b87a74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Thu, 29 Sep 2016 14:02:37 -0400 Subject: [PATCH 15/26] test: add case where axis type is set by user --- test/jasmine/tests/transform_filter_test.js | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/test/jasmine/tests/transform_filter_test.js b/test/jasmine/tests/transform_filter_test.js index ca766a73c0e..4e200ac2301 100644 --- a/test/jasmine/tests/transform_filter_test.js +++ b/test/jasmine/tests/transform_filter_test.js @@ -84,10 +84,10 @@ describe('filter transforms calc:', function() { return calcTrace[0].trace; } - function _transform(data) { + function _transform(data, layout) { var gd = { data: data, - layout: {} + layout: layout || {} }; Plots.supplyDefaults(gd); @@ -256,6 +256,21 @@ describe('filter transforms calc:', function() { ); }); + it('should honored set axis type', function() { + var out = _transform([Lib.extendDeep({}, _base, { + x: [1, 2, 3, 0, -1, -2, -3], + transforms: [{ + operation: '>', + value: -1, + filtersrc: 'x' + }] + })], { + xaxis: { type: 'category' } + }); + + _assert(out, [-2, -3], [3, 1], [0.3, 0.4]); + }); + }); describe('filters should handle categories', function() { From f4b9dcca7dfbefa1033e1c75c03c7c77be136ae6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 30 Sep 2016 15:05:24 -0400 Subject: [PATCH 16/26] transforms: replace 'active' attribute by 'enabled' --- src/transforms/filter.js | 10 +++++----- src/transforms/groupby.js | 8 ++++---- test/jasmine/tests/transform_filter_test.js | 14 +++++++------- test/jasmine/tests/transform_multi_test.js | 10 +++++----- 4 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/transforms/filter.js b/src/transforms/filter.js index ace01d1069b..338f900c100 100644 --- a/src/transforms/filter.js +++ b/src/transforms/filter.js @@ -16,11 +16,11 @@ exports.moduleType = 'transform'; exports.name = 'filter'; exports.attributes = { - active: { + enabled: { valType: 'boolean', dflt: true, description: [ - 'Toggles whether or not the filter is active.' + 'Determines whether this filter transform is enabled or disabled.' ].join(' ') }, filtersrc: { @@ -74,9 +74,9 @@ exports.supplyDefaults = function(transformIn) { return Lib.coerce(transformIn, transformOut, exports.attributes, attr, dflt); } - var active = coerce('active'); + var enabled = coerce('enabled'); - if(active) { + if(enabled) { var operation = coerce('operation'); coerce('value'); @@ -97,7 +97,7 @@ exports.transform = function(data) { exports.calcTransform = function(gd, trace, opts) { var filtersrc = opts.filtersrc; - if(!opts.active || !trace[filtersrc]) return; + if(!opts.enabled || !trace[filtersrc]) return; var dataToCoord = getDataToCoordFunc(gd, filtersrc), filterFunc = getFilterFunc(opts, dataToCoord); diff --git a/src/transforms/groupby.js b/src/transforms/groupby.js index 2af72d50175..f736c0e1a75 100644 --- a/src/transforms/groupby.js +++ b/src/transforms/groupby.js @@ -15,11 +15,11 @@ exports.moduleType = 'transform'; exports.name = 'groupby'; exports.attributes = { - active: { + enabled: { valType: 'boolean', dflt: true, description: [ - 'Toggles whether or not the filter is active.' + 'Determines whether this group-by transform is enabled or disabled.' ].join(' ') }, groups: { @@ -65,9 +65,9 @@ exports.supplyDefaults = function(transformIn) { return Lib.coerce(transformIn, transformOut, exports.attributes, attr, dflt); } - var active = coerce('active'); + var enabled = coerce('enabled'); - if(!active) return transformOut; + if(!enabled) return transformOut; coerce('groups'); coerce('style'); diff --git a/test/jasmine/tests/transform_filter_test.js b/test/jasmine/tests/transform_filter_test.js index 4e200ac2301..82dffa30cb6 100644 --- a/test/jasmine/tests/transform_filter_test.js +++ b/test/jasmine/tests/transform_filter_test.js @@ -25,18 +25,18 @@ describe('filter transforms defaults:', function() { expect(traceOut.transforms).toEqual([{ type: 'filter', - active: true, + enabled: true, operation: '=', value: 0, filtersrc: 'x' }]); }); - it('supplyTraceDefaults should not coerce attributes if active: false', function() { + it('supplyTraceDefaults should not coerce attributes if enabled: false', function() { traceIn = { x: [1, 2, 3], transforms: [{ - active: false, + enabled: false, type: 'filter', value: 0 }] @@ -46,7 +46,7 @@ describe('filter transforms defaults:', function() { expect(traceOut.transforms).toEqual([{ type: 'filter', - active: false, + enabled: false, }]); }); @@ -121,11 +121,11 @@ describe('filter transforms calc:', function() { expect(out[0].y).toEqual(base.y); }); - it('filters should skip if *active* is false', function() { + it('filters should skip if *enabled* is false', function() { var out = _transform([Lib.extendDeep({}, base, { transforms: [{ type: 'filter', - active: false, + enabled: false, operation: '>', value: 0, filtersrc: 'x' @@ -164,7 +164,7 @@ describe('filter transforms calc:', function() { filtersrc: 'x' }, { type: 'filter', - active: false, + enabled: false, operation: '>', value: 2, filtersrc: 'y' diff --git a/test/jasmine/tests/transform_multi_test.js b/test/jasmine/tests/transform_multi_test.js index fe5e7b7c734..59440cf3ce1 100644 --- a/test/jasmine/tests/transform_multi_test.js +++ b/test/jasmine/tests/transform_multi_test.js @@ -23,7 +23,7 @@ describe('general transforms:', function() { expect(traceOut.transforms).toEqual([{ type: 'filter', - active: true, + enabled: true, operation: '=', value: 0, filtersrc: 'x' @@ -62,7 +62,7 @@ describe('general transforms:', function() { expect(traceOut.transforms[0]).toEqual({ type: 'filter', - active: true, + enabled: true, operation: '=', value: 0, filtersrc: 'x' @@ -70,7 +70,7 @@ describe('general transforms:', function() { expect(traceOut.transforms[1]).toEqual({ type: 'filter', - active: true, + enabled: true, operation: '>', strictinterval: true, value: 0, @@ -111,7 +111,7 @@ describe('general transforms:', function() { msg = 'supplying the transform defaults'; expect(dataOut[1].transforms[0]).toEqual({ type: 'filter', - active: true, + enabled: true, operation: '>', value: 0, strictinterval: true, @@ -131,7 +131,7 @@ describe('general transforms:', function() { msg = 'keeping refs to full transforms array'; expect(dataOut[1]._fullInput.transforms).toEqual([{ type: 'filter', - active: true, + enabled: true, operation: '>', value: 0, strictinterval: true, From 104123ef81512863464c2cd29ed103dcf71ebf16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 30 Sep 2016 16:36:11 -0400 Subject: [PATCH 17/26] transforms: replace 'strictinterval' with MORE operation values --- src/transforms/filter.js | 201 +++++++++----------- test/jasmine/tests/transform_filter_test.js | 57 ++---- test/jasmine/tests/transform_multi_test.js | 3 - 3 files changed, 106 insertions(+), 155 deletions(-) diff --git a/src/transforms/filter.js b/src/transforms/filter.js index 338f900c100..84bfa3aa6d0 100644 --- a/src/transforms/filter.js +++ b/src/transforms/filter.js @@ -11,6 +11,10 @@ var Lib = require('../lib'); var axisIds = require('../plots/cartesian/axis_ids'); +var INEQUALITY_OPS = ['=', '<', '>=', '>', '<=']; +var INTERVAL_OPS = ['[]', '()', '[)', '(]', '][', ')(', '](', ')[']; +var SET_OPS = ['{}', '}{']; + exports.moduleType = 'transform'; exports.name = 'filter'; @@ -33,10 +37,31 @@ exports.attributes = { }, operation: { valType: 'enumerated', - values: ['=', '<', '>', 'within', 'notwithin', 'in', 'notin'], + values: [].concat(INEQUALITY_OPS).concat(INTERVAL_OPS).concat(SET_OPS), dflt: '=', description: [ - 'Sets the filter operation.' + 'Sets the filter operation.', + + '*=* filters items equal to `value`', + + '*<* filters items less than `value`', + '*<=* filters items less than or equal to `value`', + + '*>* filters items greater than `value`', + '*>=* filters items greater than or equal to `value`', + + '*[]* filters items inside `value[0]` to value[1]` including both bounds`', + '*()* filters items inside `value[0]` to value[1]` excluding both bounds`', + '*[)* filters items inside `value[0]` to value[1]` including `value[0]` but excluding `value[1]', + '*(]* filters items inside `value[0]` to value[1]` including both bounds`', + + '*][* filters items outside `value[0]` to value[1]` and not equal to both bounds`', + '*)(* filters items outside `value[0]` to value[1]`', + '*](* filters items outside `value[0]` to value[1]` and not equal to `value[0]`', + '*)[* filters items outside `value[0]` to value[1]` and not equal to `value[1]`', + + '*{}* filters items present in a set of values', + '*}{* filters items not present in a set of values' ].join(' ') }, value: { @@ -51,19 +76,6 @@ exports.attributes = { 'is the lower bound and the second item is the upper bound.', 'When `operation`, is set to *in*, *notin* ' ].join(' ') - }, - strictinterval: { - valType: 'boolean', - dflt: true, - arrayOk: true, - description: [ - 'Determines whether or not the filter operation includes data item value,', - 'equal to *value*.', - 'Has only an effect for `operation` *>*, *<*, *within* and *notwithin*', - 'When `operation` is set to *within* and *notwithin*,', - '`strictinterval` is expected to be a 2-item array where the first (second)', - 'item determines strictness for the lower (second) bound.' - ].join(' ') } }; @@ -77,14 +89,9 @@ exports.supplyDefaults = function(transformIn) { var enabled = coerce('enabled'); if(enabled) { - var operation = coerce('operation'); - + coerce('operation'); coerce('value'); coerce('filtersrc'); - - if(['=', 'in', 'notin'].indexOf(operation) === -1) { - coerce('strictinterval'); - } } return transformOut; @@ -154,33 +161,23 @@ function getDataToCoordFunc(gd, filtersrc) { function getFilterFunc(opts, d2c) { var operation = opts.operation, value = opts.value, - hasArrayValue = Array.isArray(value), - strict = opts.strictinterval, - hasArrayStrict = Array.isArray(strict); + hasArrayValue = Array.isArray(value); function isOperationIn(array) { return array.indexOf(operation) !== -1; } - var coercedValue, coercedStrict; + var coercedValue; - if(isOperationIn(['=', '<', '>'])) { + if(isOperationIn(INEQUALITY_OPS)) { coercedValue = hasArrayValue ? d2c(value[0]) : d2c(value); - - if(isOperationIn(['<', '>'])) { - coercedStrict = hasArrayStrict ? strict[0] : strict; - } } - else if(isOperationIn(['within', 'notwithin'])) { + else if(isOperationIn(INTERVAL_OPS)) { coercedValue = hasArrayValue ? [d2c(value[0]), d2c(value[1])] : [d2c(value), d2c(value)]; - - coercedStrict = hasArrayStrict ? - [strict[0], strict[1]] : - [strict, strict]; } - else if(isOperationIn(['in', 'notin'])) { + else if(isOperationIn(SET_OPS)) { coercedValue = hasArrayValue ? value.map(d2c) : [d2c(value)]; } @@ -190,85 +187,71 @@ function getFilterFunc(opts, d2c) { return function(v) { return d2c(v) === coercedValue; }; case '<': - if(coercedStrict) { - return function(v) { return d2c(v) < coercedValue; }; - } - else { - return function(v) { return d2c(v) <= coercedValue; }; - } + return function(v) { return d2c(v) < coercedValue; }; + + case '<=': + return function(v) { return d2c(v) <= coercedValue; }; case '>': - if(coercedStrict) { - return function(v) { return d2c(v) > coercedValue; }; - } - else { - return function(v) { return d2c(v) >= coercedValue; }; - } - - case 'within': - - if(coercedStrict[0] && coercedStrict[1]) { - return function(v) { - var cv = d2c(v); - return cv > coercedValue[0] && cv < coercedValue[1]; - }; - } - else if(coercedStrict[0] && !coercedStrict[1]) { - return function(v) { - var cv = d2c(v); - return cv > coercedValue[0] && cv <= coercedValue[1]; - }; - } - else if(!coercedStrict[0] && coercedStrict[1]) { - return function(v) { - var cv = d2c(v); - return cv >= coercedValue[0] && cv < coercedValue[1]; - }; - } - else if(!coercedStrict[0] && !coercedStrict[1]) { - return function(v) { - var cv = d2c(v); - return cv >= coercedValue[0] && cv <= coercedValue[1]; - }; - } - - break; - - case 'notwithin': - - if(coercedStrict[0] && coercedStrict[1]) { - return function(v) { - var cv = d2c(v); - return cv < coercedValue[0] || cv > coercedValue[1]; - }; - } - else if(coercedStrict[0] && !coercedStrict[1]) { - return function(v) { - var cv = d2c(v); - return cv < coercedValue[0] || cv >= coercedValue[1]; - }; - } - else if(!coercedStrict[0] && coercedStrict[1]) { - return function(v) { - var cv = d2c(v); - return cv <= coercedValue[0] || cv > coercedValue[1]; - }; - } - else if(!coercedStrict[0] && !coercedStrict[1]) { - return function(v) { - var cv = d2c(v); - return cv <= coercedValue[0] || cv >= coercedValue[1]; - }; - } - - break; - - case 'in': + return function(v) { return d2c(v) > coercedValue; }; + + case '>=': + return function(v) { return d2c(v) >= coercedValue; }; + + case '[]': + return function(v) { + var cv = d2c(v); + return cv >= coercedValue[0] && cv <= coercedValue[1]; + }; + + case '()': + return function(v) { + var cv = d2c(v); + return cv > coercedValue[0] && cv < coercedValue[1]; + }; + + case '[)': + return function(v) { + var cv = d2c(v); + return cv >= coercedValue[0] && cv < coercedValue[1]; + }; + + case '(]': + return function(v) { + var cv = d2c(v); + return cv > coercedValue[0] && cv <= coercedValue[1]; + }; + + case '][': + return function(v) { + var cv = d2c(v); + return cv < coercedValue[0] || cv > coercedValue[1]; + }; + + case ')(': + return function(v) { + var cv = d2c(v); + return cv <= coercedValue[0] || cv >= coercedValue[1]; + }; + + case '](': + return function(v) { + var cv = d2c(v); + return cv < coercedValue[0] || cv >= coercedValue[1]; + }; + + case ')[': + return function(v) { + var cv = d2c(v); + return cv <= coercedValue[0] || cv > coercedValue[1]; + }; + + case '{}': return function(v) { return coercedValue.indexOf(d2c(v)) !== -1; }; - case 'notin': + case '}{': return function(v) { return coercedValue.indexOf(d2c(v)) === -1; }; diff --git a/test/jasmine/tests/transform_filter_test.js b/test/jasmine/tests/transform_filter_test.js index 82dffa30cb6..cb795ebae7f 100644 --- a/test/jasmine/tests/transform_filter_test.js +++ b/test/jasmine/tests/transform_filter_test.js @@ -49,32 +49,6 @@ describe('filter transforms defaults:', function() { enabled: false, }]); }); - - it('supplyTraceDefaults should not coerce \'strictinterval\' when it has no meaning', function() { - traceIn = { - x: [1, 2, 3], - transforms: [{ - type: 'filter', - value: 0, - operation: '=', - }] - }; - - traceOut = Plots.supplyTraceDefaults(traceIn, 0, {}); - expect(traceOut.transforms[0].strictinterval).toBeUndefined(); - - Lib.extendDeep(traceIn, { transforms: [{operation: 'in'}]}); - traceOut = Plots.supplyTraceDefaults(traceIn, 0, {}); - expect(traceOut.transforms[0].strictinterval).toBeUndefined(); - - Lib.extendDeep(traceIn, { transforms: [{operation: 'notin'}]}); - traceOut = Plots.supplyTraceDefaults(traceIn, 0, {}); - expect(traceOut.transforms[0].strictinterval).toBeUndefined(); - - Lib.extendDeep(traceIn, { transforms: [{operation: '>'}]}); - traceOut = Plots.supplyTraceDefaults(traceIn, 0, {}); - expect(traceOut.transforms[0].strictinterval).toEqual(true); - }); }); describe('filter transforms calc:', function() { @@ -193,9 +167,8 @@ describe('filter transforms calc:', function() { it('+ *within*', function() { var out = _transform([Lib.extendDeep({}, _base, { transforms: [{ - operation: 'within', + operation: '[]', value: [-1, 1], - strictinterval: [false, false], filtersrc: 'x' }] })]); @@ -211,7 +184,7 @@ describe('filter transforms calc:', function() { it('+ *notwithin*', function() { var out = _transform([Lib.extendDeep({}, _base, { transforms: [{ - operation: 'notwithin', + operation: '][', value: [-1, 1], filtersrc: 'x' }] @@ -227,7 +200,7 @@ describe('filter transforms calc:', function() { it('+ *in*', function() { var out = _transform([Lib.extendDeep({}, _base, { transforms: [{ - operation: 'in', + operation: '{}', value: [-2, 0], filtersrc: 'x' }] @@ -243,7 +216,7 @@ describe('filter transforms calc:', function() { it('+ *notin*', function() { var out = _transform([Lib.extendDeep({}, _base, { transforms: [{ - operation: 'notin', + operation: '}{', value: [-2, 0], filtersrc: 'x' }] @@ -294,7 +267,7 @@ describe('filter transforms calc:', function() { it('+ *within*', function() { var out = _transform([Lib.extendDeep({}, _base, { transforms: [{ - operation: 'within', + operation: '()', value: ['a', 'c'], filtersrc: 'x' }] @@ -306,7 +279,7 @@ describe('filter transforms calc:', function() { it('+ *notwithin*', function() { var out = _transform([Lib.extendDeep({}, _base, { transforms: [{ - operation: 'notwithin', + operation: '][', value: ['a', 'c'], filtersrc: 'x' }] @@ -318,7 +291,7 @@ describe('filter transforms calc:', function() { it('filters should handle categories + *in*', function() { var out = _transform([Lib.extendDeep({}, _base, { transforms: [{ - operation: 'in', + operation: '{}', value: ['b', 'd'], filtersrc: 'x' }] @@ -330,7 +303,7 @@ describe('filter transforms calc:', function() { it('filters should handle categories + *notin*', function() { var out = _transform([Lib.extendDeep({}, _base, { transforms: [{ - operation: 'notin', + operation: '}{', value: ['b', 'd'], filtersrc: 'x' }] @@ -388,9 +361,8 @@ describe('filter transforms calc:', function() { it('+ *>*', function() { var out = _transform([Lib.extendDeep({}, _base, { transforms: [{ - operation: '>', + operation: '>=', value: '2016-08-01', - strictinterval: false, filtersrc: 'x' }] })]); @@ -405,9 +377,8 @@ describe('filter transforms calc:', function() { it('+ *within*', function() { var out = _transform([Lib.extendDeep({}, _base, { transforms: [{ - operation: 'within', + operation: '[]', value: ['2016-08-01', '2016-10-01'], - strictinterval: false, filtersrc: 'x' }] })]); @@ -418,7 +389,7 @@ describe('filter transforms calc:', function() { it('+ *notwithin*', function() { var out = _transform([Lib.extendDeep({}, _base, { transforms: [{ - operation: 'notwithin', + operation: '][', value: ['2016-08-01', '2016-10-01'], filtersrc: 'x' }] @@ -430,7 +401,7 @@ describe('filter transforms calc:', function() { it('+ *in*', function() { var out = _transform([Lib.extendDeep({}, _base, { transforms: [{ - operation: 'in', + operation: '{}', value: '2015-07-20', filtersrc: 'x' }] @@ -442,7 +413,7 @@ describe('filter transforms calc:', function() { it('+ *notin*', function() { var out = _transform([Lib.extendDeep({}, _base, { transforms: [{ - operation: 'notin', + operation: '}{', value: ['2016-08-01', '2016-09-01', '2016-10-21', '2016-12-02'], filtersrc: 'x' }] @@ -456,7 +427,7 @@ describe('filter transforms calc:', function() { it('filters should handle ids', function() { var out = _transform([Lib.extendDeep({}, base, { transforms: [{ - operation: 'in', + operation: '{}', value: ['p1', 'p2', 'n1'], filtersrc: 'ids' }] diff --git a/test/jasmine/tests/transform_multi_test.js b/test/jasmine/tests/transform_multi_test.js index 59440cf3ce1..70ac4f4ed7b 100644 --- a/test/jasmine/tests/transform_multi_test.js +++ b/test/jasmine/tests/transform_multi_test.js @@ -72,7 +72,6 @@ describe('general transforms:', function() { type: 'filter', enabled: true, operation: '>', - strictinterval: true, value: 0, filtersrc: 'x' }, '- trace second'); @@ -114,7 +113,6 @@ describe('general transforms:', function() { enabled: true, operation: '>', value: 0, - strictinterval: true, filtersrc: 'x' }, msg); @@ -134,7 +132,6 @@ describe('general transforms:', function() { enabled: true, operation: '>', value: 0, - strictinterval: true, filtersrc: 'x' }], msg); From ba9b9331a7e2a07e80da76edbbe15ceb5d756053 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 30 Sep 2016 16:45:15 -0400 Subject: [PATCH 18/26] transforms: improve attribute descriptions --- src/transforms/filter.js | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/transforms/filter.js b/src/transforms/filter.js index 84bfa3aa6d0..307030029a6 100644 --- a/src/transforms/filter.js +++ b/src/transforms/filter.js @@ -32,7 +32,8 @@ exports.attributes = { values: ['x', 'y', 'z', 'ids'], dflt: 'x', description: [ - 'Sets the variable which the filter will be applied.' + 'Sets the variable in the parent trace object', + 'by which the filter will be applied.' ].join(' ') }, operation: { @@ -69,12 +70,23 @@ exports.attributes = { dflt: 0, description: [ 'Sets the value or values by which to filter by.', + 'Values are expected to be in the same type as the data linked', 'to *filtersrc*.', - 'When `operation` is set to *within* and *notwithin*', + + 'When `operation` is set to one of the inequality values', + '(' + INEQUALITY_OPS + ')', + '*value* is expected to be number or a string.', + + 'When `operation` is set to one of the interval value', + '(' + INTERVAL_OPS + ')', '*value* is expected to be 2-item array where the first item', 'is the lower bound and the second item is the upper bound.', - 'When `operation`, is set to *in*, *notin* ' + + 'When `operation`, is set to one of the set value', + '(' + SET_OPS + ')', + '*value* is expected to be an array with as many items as', + 'the desired set elements.' ].join(' ') } }; From 7fc7dfab3add373d8bfe9bbbbe6d417d50dd636d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 30 Sep 2016 16:56:21 -0400 Subject: [PATCH 19/26] fix a few typos --- src/plots/plots.js | 2 +- src/transforms/filter.js | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/plots/plots.js b/src/plots/plots.js index 66391c6c299..8c679e9153d 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -1635,7 +1635,7 @@ plots.doCalcdata = function(gd, traces) { // call calcTransform method if any if(trace.transforms) { - // we need one round of of trace module calc before + // we need one round of trace module calc before // the calc transform to 'fill in' the categories list // used for example in the data-to-coordinate method _module = trace._module; diff --git a/src/transforms/filter.js b/src/transforms/filter.js index 307030029a6..0a73e7f5c24 100644 --- a/src/transforms/filter.js +++ b/src/transforms/filter.js @@ -54,7 +54,7 @@ exports.attributes = { '*[]* filters items inside `value[0]` to value[1]` including both bounds`', '*()* filters items inside `value[0]` to value[1]` excluding both bounds`', '*[)* filters items inside `value[0]` to value[1]` including `value[0]` but excluding `value[1]', - '*(]* filters items inside `value[0]` to value[1]` including both bounds`', + '*(]* filters items inside `value[0]` to value[1]` excluding `value[0]` but including `value[1]', '*][* filters items outside `value[0]` to value[1]` and not equal to both bounds`', '*)(* filters items outside `value[0]` to value[1]`', @@ -76,7 +76,7 @@ exports.attributes = { 'When `operation` is set to one of the inequality values', '(' + INEQUALITY_OPS + ')', - '*value* is expected to be number or a string.', + '*value* is expected to be a number or a string.', 'When `operation` is set to one of the interval value', '(' + INTERVAL_OPS + ')', @@ -166,8 +166,8 @@ function getDataToCoordFunc(gd, filtersrc) { // -> cast to String if(filtersrc === 'ids') return function(v) { return String(v); }; - // otherwise -> case to number - return function(v) { return +(v); }; + // otherwise -> cast to Number + return function(v) { return +v; }; } function getFilterFunc(opts, d2c) { From 7cfa4ffaf0ae51e14fda6d64229e7d1f18bd09a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 30 Sep 2016 17:25:07 -0400 Subject: [PATCH 20/26] transforms: fix logic for outside intervals - add a missing test cases for semi-open intervals - replace verb 'filter' by 'keep' in operation description --- src/transforms/filter.js | 38 ++++----- test/jasmine/tests/transform_filter_test.js | 93 +++++++++++++++++++-- 2 files changed, 107 insertions(+), 24 deletions(-) diff --git a/src/transforms/filter.js b/src/transforms/filter.js index 0a73e7f5c24..28f46f85998 100644 --- a/src/transforms/filter.js +++ b/src/transforms/filter.js @@ -43,26 +43,26 @@ exports.attributes = { description: [ 'Sets the filter operation.', - '*=* filters items equal to `value`', + '*=* keeps items equal to `value`', - '*<* filters items less than `value`', - '*<=* filters items less than or equal to `value`', + '*<* keeps items less than `value`', + '*<=* keeps items less than or equal to `value`', - '*>* filters items greater than `value`', - '*>=* filters items greater than or equal to `value`', + '*>* keeps items greater than `value`', + '*>=* keeps items greater than or equal to `value`', - '*[]* filters items inside `value[0]` to value[1]` including both bounds`', - '*()* filters items inside `value[0]` to value[1]` excluding both bounds`', - '*[)* filters items inside `value[0]` to value[1]` including `value[0]` but excluding `value[1]', - '*(]* filters items inside `value[0]` to value[1]` excluding `value[0]` but including `value[1]', + '*[]* keeps items inside `value[0]` to value[1]` including both bounds`', + '*()* keeps items inside `value[0]` to value[1]` excluding both bounds`', + '*[)* keeps items inside `value[0]` to value[1]` including `value[0]` but excluding `value[1]', + '*(]* keeps items inside `value[0]` to value[1]` excluding `value[0]` but including `value[1]', - '*][* filters items outside `value[0]` to value[1]` and not equal to both bounds`', - '*)(* filters items outside `value[0]` to value[1]`', - '*](* filters items outside `value[0]` to value[1]` and not equal to `value[0]`', - '*)[* filters items outside `value[0]` to value[1]` and not equal to `value[1]`', + '*][* keeps items outside `value[0]` to value[1]` and equal to both bounds`', + '*)(* keeps items outside `value[0]` to value[1]`', + '*](* keeps items outside `value[0]` to value[1]` and equal to `value[0]`', + '*)[* keeps items outside `value[0]` to value[1]` and equal to `value[1]`', - '*{}* filters items present in a set of values', - '*}{* filters items not present in a set of values' + '*{}* keeps items present in a set of values', + '*}{* keeps items not present in a set of values' ].join(' ') }, value: { @@ -237,25 +237,25 @@ function getFilterFunc(opts, d2c) { case '][': return function(v) { var cv = d2c(v); - return cv < coercedValue[0] || cv > coercedValue[1]; + return cv <= coercedValue[0] || cv >= coercedValue[1]; }; case ')(': return function(v) { var cv = d2c(v); - return cv <= coercedValue[0] || cv >= coercedValue[1]; + return cv < coercedValue[0] || cv > coercedValue[1]; }; case '](': return function(v) { var cv = d2c(v); - return cv < coercedValue[0] || cv >= coercedValue[1]; + return cv <= coercedValue[0] || cv > coercedValue[1]; }; case ')[': return function(v) { var cv = d2c(v); - return cv <= coercedValue[0] || cv > coercedValue[1]; + return cv < coercedValue[0] || cv >= coercedValue[1]; }; case '{}': diff --git a/test/jasmine/tests/transform_filter_test.js b/test/jasmine/tests/transform_filter_test.js index cb795ebae7f..70fd0a67208 100644 --- a/test/jasmine/tests/transform_filter_test.js +++ b/test/jasmine/tests/transform_filter_test.js @@ -173,18 +173,53 @@ describe('filter transforms calc:', function() { }] })]); - _assert( - out, + _assert(out, [-1, 0, 1], [2, 1, 2], [0.2, 0.1, 0.2] ); }); + it('+ *within* + with RHS open', function() { + var out = _transform([Lib.extendDeep({}, _base, { + transforms: [{ + operation: '[)', + value: [-1, 1], + filtersrc: 'x' + }] + })]); + + _assert(out, [-1, 0], [2, 1], [0.2, 0.1]); + }); + + it('+ *within* + with LHS open', function() { + var out = _transform([Lib.extendDeep({}, _base, { + transforms: [{ + operation: '(]', + value: [-1, 1], + filtersrc: 'x' + }] + })]); + + _assert(out, [0, 1], [1, 2], [0.1, 0.2]); + }); + + it('+ *within* + with both sides open', function() { + var out = _transform([Lib.extendDeep({}, _base, { + transforms: [{ + operation: '()', + value: [-1, 1], + filtersrc: 'x' + }] + })]); + + _assert(out, [0], [1], [0.1]); + }); + it('+ *notwithin*', function() { var out = _transform([Lib.extendDeep({}, _base, { transforms: [{ - operation: '][', + operation: ')(', value: [-1, 1], filtersrc: 'x' }] @@ -197,6 +232,54 @@ describe('filter transforms calc:', function() { ); }); + it('+ *notwithin* with RHS closed', function() { + var out = _transform([Lib.extendDeep({}, _base, { + transforms: [{ + operation: ')[', + value: [-1, 1], + filtersrc: 'x' + }] + })]); + + _assert(out, + [-2, -2, 1, 2, 3], + [1, 3, 2, 3, 1], + [0.1, 0.3, 0.2, 0.3, 0.4] + ); + }); + + it('+ *notwithin* with LHS closed', function() { + var out = _transform([Lib.extendDeep({}, _base, { + transforms: [{ + operation: '](', + value: [-1, 1], + filtersrc: 'x' + }] + })]); + + _assert(out, + [-2, -1, -2, 2, 3], + [1, 2, 3, 3, 1], + [0.1, 0.2, 0.3, 0.3, 0.4] + ); + }); + + it('+ *notwithin* with both sides closed', function() { + var out = _transform([Lib.extendDeep({}, _base, { + transforms: [{ + operation: '][', + value: [-1, 1], + filtersrc: 'x' + }] + })]); + + _assert(out, + [-2, -1, -2, 1, 2, 3], + [1, 2, 3, 2, 3, 1], + [0.1, 0.2, 0.3, 0.2, 0.3, 0.4] + ); + }); + it('+ *in*', function() { var out = _transform([Lib.extendDeep({}, _base, { transforms: [{ @@ -279,7 +362,7 @@ describe('filter transforms calc:', function() { it('+ *notwithin*', function() { var out = _transform([Lib.extendDeep({}, _base, { transforms: [{ - operation: '][', + operation: ')(', value: ['a', 'c'], filtersrc: 'x' }] @@ -389,7 +472,7 @@ describe('filter transforms calc:', function() { it('+ *notwithin*', function() { var out = _transform([Lib.extendDeep({}, _base, { transforms: [{ - operation: '][', + operation: ')(', value: ['2016-08-01', '2016-10-01'], filtersrc: 'x' }] From 3e74c8010b3c14b81ab2c17be9fbbfa343a2720e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 3 Oct 2016 11:07:36 -0400 Subject: [PATCH 21/26] api: relax transform module requirements - transform module are now required to have a transfrom OR calcTransform method --- src/plot_api/register.js | 10 ++++++++-- src/plots/plots.js | 2 +- src/transforms/filter.js | 4 ---- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/plot_api/register.js b/src/plot_api/register.js index 4451bc17020..f7d14efd24d 100644 --- a/src/plot_api/register.js +++ b/src/plot_api/register.js @@ -61,8 +61,14 @@ function registerTransformModule(newModule) { var prefix = 'Transform module ' + newModule.name; - if(typeof newModule.transform !== 'function') { - throw new Error(prefix + ' is missing a *transform* function.'); + var hasTransform = typeof newModule.transform === 'function', + hasCalcTransform = typeof newModule.calcTransform === 'function'; + + + if(!hasTransform && !hasCalcTransform) { + throw new Error(prefix + ' is missing a *transform* or *calcTransform* method.'); + } + } if(!Lib.isPlainObject(newModule.attributes)) { Lib.log(prefix + ' registered without an *attributes* object.'); diff --git a/src/plots/plots.js b/src/plots/plots.js index 8c679e9153d..7140900068a 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -792,7 +792,7 @@ function applyTransforms(fullTrace, fullData, layout) { var transform = container[i], _module = transformsRegistry[transform.type]; - if(_module) { + if(_module && _module.transform) { dataOut = _module.transform(dataOut, { transform: transform, fullTrace: fullTrace, diff --git a/src/transforms/filter.js b/src/transforms/filter.js index 28f46f85998..f19dfc3f228 100644 --- a/src/transforms/filter.js +++ b/src/transforms/filter.js @@ -109,10 +109,6 @@ exports.supplyDefaults = function(transformIn) { return transformOut; }; -exports.transform = function(data) { - return data; -}; - exports.calcTransform = function(gd, trace, opts) { var filtersrc = opts.filtersrc; From 2a7738e70ce103d0e4a5b34c97cea3c04403336c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 3 Oct 2016 11:08:06 -0400 Subject: [PATCH 22/26] api: improve register transform logging and tests --- src/plot_api/register.js | 12 +++++++- test/jasmine/tests/register_test.js | 43 +++++++++++++++++++++++++++-- 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/src/plot_api/register.js b/src/plot_api/register.js index f7d14efd24d..9b652572592 100644 --- a/src/plot_api/register.js +++ b/src/plot_api/register.js @@ -69,12 +69,22 @@ function registerTransformModule(newModule) { throw new Error(prefix + ' is missing a *transform* or *calcTransform* method.'); } + // For more info, see: + // https://github.com/plotly/plotly.js/pull/978#pullrequestreview-2403353 + if(hasTransform && hasCalcTransform) { + Lib.log([ + prefix + ' has both a *transform* and *calcTransform* methods.', + 'Please note that all *transform* methods are executed', + 'before all *calcTransform* methods.' + ].join(' ')); } + if(!Lib.isPlainObject(newModule.attributes)) { Lib.log(prefix + ' registered without an *attributes* object.'); } + if(typeof newModule.supplyDefaults !== 'function') { - Lib.log(prefix + ' registered without a *supplyDefaults* function.'); + Lib.log(prefix + ' registered without a *supplyDefaults* method.'); } Registry.transformsRegistry[newModule.name] = newModule; diff --git a/test/jasmine/tests/register_test.js b/test/jasmine/tests/register_test.js index 9d9df33bdb0..6fa534a2e42 100644 --- a/test/jasmine/tests/register_test.js +++ b/test/jasmine/tests/register_test.js @@ -206,9 +206,11 @@ describe('the register function', function() { expect(function() { Plotly.register([invalidTrace]); }).toThrowError(Error, 'Invalid module was attempted to be registered!'); + + expect(Registry.transformsRegistry['mah-transform']).toBeUndefined(); }); - it('should throw when if transform module is invalid', function() { + it('should throw when if transform module is invalid (1)', function() { var missingTransformName = { moduleType: 'transform' }; @@ -217,6 +219,10 @@ describe('the register function', function() { Plotly.register(missingTransformName); }).toThrowError(Error, 'Transform module *name* must be a string.'); + expect(Registry.transformsRegistry['mah-transform']).toBeUndefined(); + }); + + it('should throw when if transform module is invalid (2)', function() { var missingTransformFunc = { moduleType: 'transform', name: 'mah-transform' @@ -224,8 +230,12 @@ describe('the register function', function() { expect(function() { Plotly.register(missingTransformFunc); - }).toThrowError(Error, 'Transform module mah-transform is missing a *transform* function.'); + }).toThrowError(Error, 'Transform module mah-transform is missing a *transform* or *calcTransform* method.'); + + expect(Registry.transformsRegistry['mah-transform']).toBeUndefined(); + }); + it('should not throw when transform module is valid (1)', function() { var transformModule = { moduleType: 'transform', name: 'mah-transform', @@ -238,4 +248,33 @@ describe('the register function', function() { expect(Registry.transformsRegistry['mah-transform']).toBeDefined(); }); + + it('should not throw when transform module is valid (2)', function() { + var transformModule = { + moduleType: 'transform', + name: 'mah-transform', + calcTransform: function() {} + }; + + expect(function() { + Plotly.register(transformModule); + }).not.toThrow(); + + expect(Registry.transformsRegistry['mah-transform']).toBeDefined(); + }); + + it('should not throw when transform module is valid (3)', function() { + var transformModule = { + moduleType: 'transform', + name: 'mah-transform', + transform: function() {}, + calcTransform: function() {} + }; + + expect(function() { + Plotly.register(transformModule); + }).not.toThrow(); + + expect(Registry.transformsRegistry['mah-transform']).toBeDefined(); + }); }); From 0aa495824f3c4154fa5cc2a7e5a135914a162cf7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 3 Oct 2016 11:15:11 -0400 Subject: [PATCH 23/26] test: update transform case descriptions --- test/jasmine/tests/transform_filter_test.js | 42 ++++++++++----------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/test/jasmine/tests/transform_filter_test.js b/test/jasmine/tests/transform_filter_test.js index 70fd0a67208..674727601f0 100644 --- a/test/jasmine/tests/transform_filter_test.js +++ b/test/jasmine/tests/transform_filter_test.js @@ -164,7 +164,7 @@ describe('filter transforms calc:', function() { expect(out[0].marker.size).toEqual(20, '- marker.size style'); } - it('+ *within*', function() { + it('with operation *[]*', function() { var out = _transform([Lib.extendDeep({}, _base, { transforms: [{ operation: '[]', @@ -180,7 +180,7 @@ describe('filter transforms calc:', function() { ); }); - it('+ *within* + with RHS open', function() { + it('with operation *[)*', function() { var out = _transform([Lib.extendDeep({}, _base, { transforms: [{ operation: '[)', @@ -192,7 +192,7 @@ describe('filter transforms calc:', function() { _assert(out, [-1, 0], [2, 1], [0.2, 0.1]); }); - it('+ *within* + with LHS open', function() { + it('with operation *(]*', function() { var out = _transform([Lib.extendDeep({}, _base, { transforms: [{ operation: '(]', @@ -204,7 +204,7 @@ describe('filter transforms calc:', function() { _assert(out, [0, 1], [1, 2], [0.1, 0.2]); }); - it('+ *within* + with both sides open', function() { + it('with operation *()*', function() { var out = _transform([Lib.extendDeep({}, _base, { transforms: [{ operation: '()', @@ -216,7 +216,7 @@ describe('filter transforms calc:', function() { _assert(out, [0], [1], [0.1]); }); - it('+ *notwithin*', function() { + it('with operation *)(*', function() { var out = _transform([Lib.extendDeep({}, _base, { transforms: [{ operation: ')(', @@ -232,7 +232,7 @@ describe('filter transforms calc:', function() { ); }); - it('+ *notwithin* with RHS closed', function() { + it('with operation *)[*', function() { var out = _transform([Lib.extendDeep({}, _base, { transforms: [{ operation: ')[', @@ -248,7 +248,7 @@ describe('filter transforms calc:', function() { ); }); - it('+ *notwithin* with LHS closed', function() { + it('with operation *](*', function() { var out = _transform([Lib.extendDeep({}, _base, { transforms: [{ operation: '](', @@ -264,7 +264,7 @@ describe('filter transforms calc:', function() { ); }); - it('+ *notwithin* with both sides closed', function() { + it('with operation *][*', function() { var out = _transform([Lib.extendDeep({}, _base, { transforms: [{ operation: '][', @@ -280,7 +280,7 @@ describe('filter transforms calc:', function() { ); }); - it('+ *in*', function() { + it('with operation *{}*', function() { var out = _transform([Lib.extendDeep({}, _base, { transforms: [{ operation: '{}', @@ -296,7 +296,7 @@ describe('filter transforms calc:', function() { ); }); - it('+ *notin*', function() { + it('with operation *}{*', function() { var out = _transform([Lib.extendDeep({}, _base, { transforms: [{ operation: '}{', @@ -347,7 +347,7 @@ describe('filter transforms calc:', function() { expect(out[0].marker.color).toEqual('red', '- marker.color style'); } - it('+ *within*', function() { + it('with operation *()*', function() { var out = _transform([Lib.extendDeep({}, _base, { transforms: [{ operation: '()', @@ -359,7 +359,7 @@ describe('filter transforms calc:', function() { _assert(out, ['b'], [2], ['1']); }); - it('+ *notwithin*', function() { + it('with operation *)(*', function() { var out = _transform([Lib.extendDeep({}, _base, { transforms: [{ operation: ')(', @@ -371,7 +371,7 @@ describe('filter transforms calc:', function() { _assert(out, ['d'], [4], ['0']); }); - it('filters should handle categories + *in*', function() { + it('with operation *{}*', function() { var out = _transform([Lib.extendDeep({}, _base, { transforms: [{ operation: '{}', @@ -383,7 +383,7 @@ describe('filter transforms calc:', function() { _assert(out, ['b', 'd'], [2, 4], ['1', '0']); }); - it('filters should handle categories + *notin*', function() { + it('with operation *}{*', function() { var out = _transform([Lib.extendDeep({}, _base, { transforms: [{ operation: '}{', @@ -417,7 +417,7 @@ describe('filter transforms calc:', function() { expect(out[0].marker.line.width).toEqual(2.5, '- marker.line.width style'); } - it('+ *=*', function() { + it('with operation *=*', function() { var out = _transform([Lib.extendDeep({}, _base, { transforms: [{ operation: '=', @@ -429,7 +429,7 @@ describe('filter transforms calc:', function() { _assert(out, ['2015-07-20'], [1], [0.1]); }); - it('+ *<*', function() { + it('with operation *<*', function() { var out = _transform([Lib.extendDeep({}, _base, { transforms: [{ operation: '<', @@ -441,7 +441,7 @@ describe('filter transforms calc:', function() { _assert(out, ['2015-07-20'], [1], [0.1]); }); - it('+ *>*', function() { + it('with operation *>*', function() { var out = _transform([Lib.extendDeep({}, _base, { transforms: [{ operation: '>=', @@ -457,7 +457,7 @@ describe('filter transforms calc:', function() { ); }); - it('+ *within*', function() { + it('with operation *[]*', function() { var out = _transform([Lib.extendDeep({}, _base, { transforms: [{ operation: '[]', @@ -469,7 +469,7 @@ describe('filter transforms calc:', function() { _assert(out, ['2016-08-01', '2016-09-01'], [2, 3], [0.2, 0.3]); }); - it('+ *notwithin*', function() { + it('with operation *)(*', function() { var out = _transform([Lib.extendDeep({}, _base, { transforms: [{ operation: ')(', @@ -481,7 +481,7 @@ describe('filter transforms calc:', function() { _assert(out, ['2015-07-20', '2016-10-21', '2016-12-02'], [1, 1, 5], [0.1, 0.1, 0.2]); }); - it('+ *in*', function() { + it('with operation *{}*', function() { var out = _transform([Lib.extendDeep({}, _base, { transforms: [{ operation: '{}', @@ -493,7 +493,7 @@ describe('filter transforms calc:', function() { _assert(out, ['2015-07-20'], [1], [0.1]); }); - it('+ *notin*', function() { + it('with operation *}{*', function() { var out = _transform([Lib.extendDeep({}, _base, { transforms: [{ operation: '}{', From ad77818290251ecdc3beddd4c06bdd4990e376ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 3 Oct 2016 11:53:44 -0400 Subject: [PATCH 24/26] transforms: allow 'filtersrc' to be an arbitrary attr string - so that filters can be applied to ALL possible data array e.g `filtersrc: 'marker.color'` - add tests for geo 'lon' / 'lat' and 'marker.color' --- src/transforms/filter.js | 18 +++-- test/jasmine/tests/transform_filter_test.js | 74 +++++++++++++++++++++ 2 files changed, 86 insertions(+), 6 deletions(-) diff --git a/src/transforms/filter.js b/src/transforms/filter.js index f19dfc3f228..bf01ecbe1e7 100644 --- a/src/transforms/filter.js +++ b/src/transforms/filter.js @@ -28,12 +28,17 @@ exports.attributes = { ].join(' ') }, filtersrc: { - valType: 'enumerated', - values: ['x', 'y', 'z', 'ids'], + valType: 'string', + strict: true, + noBlank: true, dflt: 'x', description: [ 'Sets the variable in the parent trace object', - 'by which the filter will be applied.' + 'by which the filter will be applied.', + + 'To filter about nested variables, use *.* to access them.', + 'For example, set `filtersrc` to *marker.color* to filter', + 'about the marker color array.' ].join(' ') }, operation: { @@ -110,14 +115,15 @@ exports.supplyDefaults = function(transformIn) { }; exports.calcTransform = function(gd, trace, opts) { - var filtersrc = opts.filtersrc; + var filtersrc = opts.filtersrc, + filtersrcOk = filtersrc && Array.isArray(Lib.nestedProperty(trace, filtersrc).get()); - if(!opts.enabled || !trace[filtersrc]) return; + if(!opts.enabled || !filtersrcOk) return; var dataToCoord = getDataToCoordFunc(gd, filtersrc), filterFunc = getFilterFunc(opts, dataToCoord); - var filterArr = trace[filtersrc], + var filterArr = Lib.nestedProperty(trace, filtersrc).get(), len = filterArr.length; var arrayAttrs = Lib.findArrayAttributes(trace), diff --git a/test/jasmine/tests/transform_filter_test.js b/test/jasmine/tests/transform_filter_test.js index 674727601f0..ac06c2262ed 100644 --- a/test/jasmine/tests/transform_filter_test.js +++ b/test/jasmine/tests/transform_filter_test.js @@ -49,6 +49,31 @@ describe('filter transforms defaults:', function() { enabled: false, }]); }); + + it('supplyTraceDefaults should coerce *filtersrc* as a strict / noBlank string', function() { + traceIn = { + x: [1, 2, 3], + transforms: [{ + type: 'filter', + }, { + type: 'filter', + filtersrc: 0 + }, { + type: 'filter', + filtersrc: '' + }, { + type: 'filter', + filtersrc: 'marker.color' + }] + }; + + traceOut = Plots.supplyTraceDefaults(traceIn, 0, {}); + + expect(traceOut.transforms[0].filtersrc).toEqual('x'); + expect(traceOut.transforms[1].filtersrc).toEqual('x'); + expect(traceOut.transforms[2].filtersrc).toEqual('x'); + expect(traceOut.transforms[3].filtersrc).toEqual('marker.color'); + }); }); describe('filter transforms calc:', function() { @@ -95,6 +120,55 @@ describe('filter transforms calc:', function() { expect(out[0].y).toEqual(base.y); }); + it('filters should handle geographical *lon* data', function() { + var trace0 = { + type: 'scattergeo', + lon: [-90, -40, 100, 120, 130], + lat: [-50, -40, 10, 20, 30], + transforms: [{ + type: 'filter', + operation: '>', + value: 0, + filtersrc: 'lon' + }] + }; + + var trace1 = { + type: 'scattermapbox', + lon: [-90, -40, 100, 120, 130], + lat: [-50, -40, 10, 20, 30], + transforms: [{ + type: 'filter', + operation: '<', + value: 0, + filtersrc: 'lat' + }] + }; + + var out = _transform([trace0, trace1]); + + expect(out[0].lon).toEqual([100, 120, 130]); + expect(out[0].lat).toEqual([10, 20, 30]); + + expect(out[1].lon).toEqual([-90, -40]); + expect(out[1].lat).toEqual([-50, -40]); + }); + + it('filters should handle nested attributes', function() { + var out = _transform([Lib.extendDeep({}, base, { + transforms: [{ + type: 'filter', + operation: '>', + value: 0.2, + filtersrc: 'marker.color' + }] + })]); + + expect(out[0].x).toEqual([-2, 2, 3]); + expect(out[0].y).toEqual([3, 3, 1]); + expect(out[0].marker.color).toEqual([0.3, 0.3, 0.4]); + }); + it('filters should skip if *enabled* is false', function() { var out = _transform([Lib.extendDeep({}, base, { transforms: [{ From 4cd9edfb14f887f995b252b2fd245d62149db8a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 3 Oct 2016 11:54:32 -0400 Subject: [PATCH 25/26] transforms: add support for date 3D z-axes - using axisIds.getFromTrace ! --- src/transforms/filter.js | 9 +++++---- test/jasmine/tests/transform_filter_test.js | 17 +++++++++++++++++ 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/src/transforms/filter.js b/src/transforms/filter.js index bf01ecbe1e7..042798ac4ef 100644 --- a/src/transforms/filter.js +++ b/src/transforms/filter.js @@ -120,7 +120,7 @@ exports.calcTransform = function(gd, trace, opts) { if(!opts.enabled || !filtersrcOk) return; - var dataToCoord = getDataToCoordFunc(gd, filtersrc), + var dataToCoord = getDataToCoordFunc(gd, trace, filtersrc), filterFunc = getFilterFunc(opts, dataToCoord); var filterArr = Lib.nestedProperty(trace, filtersrc).get(), @@ -157,8 +157,8 @@ exports.calcTransform = function(gd, trace, opts) { } }; -function getDataToCoordFunc(gd, filtersrc) { - var ax = axisIds.getFromId(gd, filtersrc); +function getDataToCoordFunc(gd, trace, filtersrc) { + var ax = axisIds.getFromTrace(gd, trace, filtersrc); // if 'filtersrc' has corresponding axis // -> use setConvert method @@ -168,7 +168,8 @@ function getDataToCoordFunc(gd, filtersrc) { // -> cast to String if(filtersrc === 'ids') return function(v) { return String(v); }; - // otherwise -> cast to Number + // otherwise + // -> cast to Number return function(v) { return +v; }; } diff --git a/test/jasmine/tests/transform_filter_test.js b/test/jasmine/tests/transform_filter_test.js index ac06c2262ed..fdba96d7609 100644 --- a/test/jasmine/tests/transform_filter_test.js +++ b/test/jasmine/tests/transform_filter_test.js @@ -120,6 +120,23 @@ describe('filter transforms calc:', function() { expect(out[0].y).toEqual(base.y); }); + it('filters should handle 3D *z* data', function() { + var out = _transform([Lib.extendDeep({}, base, { + type: 'scatter3d', + z: ['2015-07-20', '2016-08-01', '2016-09-01', '2016-10-21', '2016-12-02'], + transforms: [{ + type: 'filter', + operation: '>', + value: '2016-10-01', + filtersrc: 'z' + }] + })]); + + expect(out[0].x).toEqual([0, 1]); + expect(out[0].y).toEqual([1, 2]); + expect(out[0].z).toEqual(['2016-10-21', '2016-12-02']); + }); + it('filters should handle geographical *lon* data', function() { var trace0 = { type: 'scattergeo', From f5c64d286f76b8aa763d49d2e4dd13304e77b662 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 5 Oct 2016 18:25:37 -0400 Subject: [PATCH 26/26] doc: add comment about transform in main index file --- lib/index.js | 9 +++++++++ src/plot_api/register.js | 2 -- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/index.js b/lib/index.js index 177c72d6102..359de92a48e 100644 --- a/lib/index.js +++ b/lib/index.js @@ -32,6 +32,15 @@ Plotly.register([ ]); // transforms +// +// Please note that all *transform* methods are executed before +// all *calcTransform* methods - which could possibly lead to +// unexpected results when applying multiple transforms of different types +// to a given trace. +// +// For more info, see: +// https://github.com/plotly/plotly.js/pull/978#pullrequestreview-2403353 +// Plotly.register([ require('./filter'), require('./groupby') diff --git a/src/plot_api/register.js b/src/plot_api/register.js index 9b652572592..5192b95a99c 100644 --- a/src/plot_api/register.js +++ b/src/plot_api/register.js @@ -69,8 +69,6 @@ function registerTransformModule(newModule) { throw new Error(prefix + ' is missing a *transform* or *calcTransform* method.'); } - // For more info, see: - // https://github.com/plotly/plotly.js/pull/978#pullrequestreview-2403353 if(hasTransform && hasCalcTransform) { Lib.log([ prefix + ' has both a *transform* and *calcTransform* methods.',