Skip to content

Filter fixes #1062

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Oct 24, 2016
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/lib/coerce.js
Original file line number Diff line number Diff line change
@@ -458,6 +458,17 @@ exports.findArrayAttributes = function(trace) {

exports.crawl(trace._module.attributes, callback);

if(trace.transforms) {
var transforms = trace.transforms;

for(var i = 0; i < transforms.length; i++) {
var transform = transforms[i];

stack = ['transforms[' + i + ']'];
exports.crawl(transform._module.attributes, callback, 1);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Loop through transform container found in the trace, use the transform module attributes declaration to determine which attributes are data_array or arrayOk.

}
}

// Look into the fullInput module attributes for array attributes
// to make sure that 'custom' array attributes are detected.
//
18 changes: 18 additions & 0 deletions src/plot_api/helpers.js
Original file line number Diff line number Diff line change
@@ -340,6 +340,24 @@ exports.cleanData = function(data, existingData) {
}
}

// transforms backward compatibility fixes
if(Array.isArray(trace.transforms)) {
var transforms = trace.transforms;

for(i = 0; i < transforms.length; i++) {
var transform = transforms[i];

if(!Lib.isPlainObject(transform)) continue;

if(transform.type === 'filter') {
if(transform.filtersrc) {
transform.target = transform.filtersrc;
delete transform.filtersrc;
}
}
}
}

// prune empty containers made before the new nestedProperty
if(emptyContainer(trace, 'line')) delete trace.line;
if('marker' in trace) {
1 change: 1 addition & 0 deletions src/plots/plots.js
Original file line number Diff line number Diff line change
@@ -807,6 +807,7 @@ function supplyTransformDefaults(traceIn, traceOut, layout) {
if(_module && _module.supplyDefaults) {
transformOut = _module.supplyDefaults(transformIn, traceOut, layout, traceIn);
transformOut.type = type;
transformOut._module = _module;
}
else {
transformOut = Lib.extendFlat({}, transformIn);
74 changes: 52 additions & 22 deletions src/transforms/filter.js
Original file line number Diff line number Diff line change
@@ -10,6 +10,8 @@

var Lib = require('../lib');
var axisIds = require('../plots/cartesian/axis_ids');
var autoType = require('../plots/cartesian/axis_autotype');
var setConvert = require('../plots/cartesian/set_convert');

var INEQUALITY_OPS = ['=', '<', '>=', '>', '<='];
var INTERVAL_OPS = ['[]', '()', '[)', '(]', '][', ')(', '](', ')['];
@@ -27,18 +29,22 @@ exports.attributes = {
'Determines whether this filter transform is enabled or disabled.'
].join(' ')
},
filtersrc: {
target: {
valType: 'string',
strict: true,
noBlank: true,
arrayOk: true,
dflt: 'x',
description: [
'Sets the variable in the parent trace object',
'by which the filter will be applied.',
'Sets the filter target by which the filter is applied.',

'If a string, *target* is assumed to be a reference to a data array',
'in the parent trace object.',
'To filter about nested variables, use *.* to access them.',
'For example, set `filtersrc` to *marker.color* to filter',
'about the marker color array.'
'For example, set `target` to *marker.color* to filter',
'about the marker color array.',

'If an array, *target* is then the data array by which the filter is applied.'
].join(' ')
},
operation: {
@@ -77,7 +83,7 @@ exports.attributes = {
'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*.',
'to *target*.',

'When `operation` is set to one of the inequality values',
'(' + INEQUALITY_OPS + ')',
@@ -108,25 +114,24 @@ exports.supplyDefaults = function(transformIn) {
if(enabled) {
coerce('operation');
coerce('value');
coerce('filtersrc');
coerce('target');
}

return transformOut;
};

exports.calcTransform = function(gd, trace, opts) {
var filtersrc = opts.filtersrc,
filtersrcOk = filtersrc && Array.isArray(Lib.nestedProperty(trace, filtersrc).get());

if(!opts.enabled || !filtersrcOk) return;
if(!opts.enabled) return;

var dataToCoord = getDataToCoordFunc(gd, trace, filtersrc),
filterFunc = getFilterFunc(opts, dataToCoord);
var target = opts.target,
filterArray = getFilterArray(trace, target),
len = filterArray.length;

var filterArr = Lib.nestedProperty(trace, filtersrc).get(),
len = filterArr.length;
if(!len) return;

var arrayAttrs = Lib.findArrayAttributes(trace),
var dataToCoord = getDataToCoordFunc(gd, trace, target),
filterFunc = getFilterFunc(opts, dataToCoord),
arrayAttrs = Lib.findArrayAttributes(trace),
originalArrays = {};

// copy all original array attribute values,
@@ -147,7 +152,7 @@ exports.calcTransform = function(gd, trace, opts) {
}

for(var i = 0; i < len; i++) {
var v = filterArr[i];
var v = filterArray[i];

if(!filterFunc(v)) continue;

@@ -157,18 +162,43 @@ exports.calcTransform = function(gd, trace, opts) {
}
};

function getDataToCoordFunc(gd, trace, filtersrc) {
var ax = axisIds.getFromTrace(gd, trace, filtersrc);
function getFilterArray(trace, target) {
if(typeof target === 'string' && target) {
var array = Lib.nestedProperty(trace, target).get();

return Array.isArray(array) ? array : [];
}
else if(Array.isArray(target)) return target.slice();

return false;
}

function getDataToCoordFunc(gd, trace, target) {
var ax;

// In the case of an array target, make a mock data array
// and call supplyDefaults to the data type and
// setup the data-to-calc method.
if(Array.isArray(target)) {
ax = {
type: autoType(target),
_categories: []
};
setConvert(ax);
}
else {
ax = axisIds.getFromTrace(gd, trace, target);
}

// if 'filtersrc' has corresponding axis
// if 'target' 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); };
if(target === 'ids') return function(v) { return String(v); };

// otherwise
// otherwise (e.g. numeric-array of 'marker.color' or 'marker.size')
// -> cast to Number
return function(v) { return +v; };
}
4 changes: 2 additions & 2 deletions test/jasmine/tests/finance_test.js
Original file line number Diff line number Diff line change
@@ -423,7 +423,7 @@ describe('finance charts calc transforms:', function() {
transforms: [{
type: 'filter',
operation: '>',
filtersrc: 'open',
target: 'open',
value: 33
}]
});
@@ -433,7 +433,7 @@ describe('finance charts calc transforms:', function() {
transforms: [{
type: 'filter',
operation: '{}',
filtersrc: 'x',
target: 'x',
value: ['2016-09-01', '2016-09-10']
}]
});
29 changes: 29 additions & 0 deletions test/jasmine/tests/plot_api_test.js
Original file line number Diff line number Diff line change
@@ -995,6 +995,35 @@ describe('Test plot api', function() {

expect(gd.data[1].contours).toBeUndefined();
});

it('should rename *filtersrc* to *target* in filter transforms', function() {
var data = [{
transforms: [{
type: 'filter',
filtersrc: 'y'
}, {
type: 'filter',
operation: '<'
}]
}, {
transforms: [{
type: 'filter',
target: 'y'
}]
}];

Plotly.plot(gd, data);

var trace0 = gd.data[0],
trace1 = gd.data[1];

expect(trace0.transforms.length).toEqual(2);
expect(trace0.transforms[0].filtersrc).toBeUndefined();
expect(trace0.transforms[0].target).toEqual('y');

expect(trace1.transforms.length).toEqual(1);
expect(trace1.transforms[0].target).toEqual('y');
});
});

describe('Plotly.update should', function() {
2 changes: 1 addition & 1 deletion test/jasmine/tests/plotschema_test.js
Original file line number Diff line number Diff line change
@@ -176,7 +176,7 @@ describe('plot schema', function() {
var valObjects = plotSchema.transforms.filter.attributes,
attrNames = Object.keys(valObjects);

['operation', 'value', 'filtersrc'].forEach(function(k) {
['operation', 'value', 'target'].forEach(function(k) {
expect(attrNames).toContain(k);
});
});
161 changes: 116 additions & 45 deletions test/jasmine/tests/transform_filter_test.js

Large diffs are not rendered by default.

25 changes: 16 additions & 9 deletions test/jasmine/tests/transform_multi_test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
var Plotly = require('@lib/index');
var Filter = require('@lib/filter');

var Plots = require('@src/plots/plots');
var Lib = require('@src/lib');

@@ -26,7 +28,8 @@ describe('general transforms:', function() {
enabled: true,
operation: '=',
value: 0,
filtersrc: 'x'
target: 'x',
_module: Filter
}]);
});

@@ -48,7 +51,7 @@ describe('general transforms:', function() {
type: 'filter',
operation: '>',
value: 0,
filtersrc: 'x'
target: 'x'
}]
};

@@ -65,15 +68,17 @@ describe('general transforms:', function() {
enabled: true,
operation: '=',
value: 0,
filtersrc: 'x'
target: 'x',
_module: Filter
}, '- global first');

expect(traceOut.transforms[1]).toEqual({
type: 'filter',
enabled: true,
operation: '>',
value: 0,
filtersrc: 'x'
target: 'x',
_module: Filter
}, '- trace second');
});

@@ -88,7 +93,7 @@ describe('general transforms:', function() {
type: 'filter',
operation: '>',
value: 0,
filtersrc: 'x'
target: 'x'
}]
}];

@@ -104,7 +109,7 @@ describe('general transforms:', function() {
type: 'filter',
operation: '>',
value: 0,
filtersrc: 'x'
target: 'x'
}], msg);

msg = 'supplying the transform defaults';
@@ -113,7 +118,8 @@ describe('general transforms:', function() {
enabled: true,
operation: '>',
value: 0,
filtersrc: 'x'
target: 'x',
_module: Filter
}, msg);

msg = 'keeping refs to user data';
@@ -123,7 +129,7 @@ describe('general transforms:', function() {
type: 'filter',
operation: '>',
value: 0,
filtersrc: 'x'
target: 'x',
}], msg);

msg = 'keeping refs to full transforms array';
@@ -132,7 +138,8 @@ describe('general transforms:', function() {
enabled: true,
operation: '>',
value: 0,
filtersrc: 'x'
target: 'x',
_module: Filter
}], msg);

msg = 'setting index w.r.t user data';
14 changes: 7 additions & 7 deletions test/jasmine/tests/transition_test.js
Original file line number Diff line number Diff line change
@@ -71,17 +71,17 @@ function runTests(transitionDuration) {
enabled: true,
type: 'filter',
operation: '<',
filtersrc: 'x',
target: 'x',
value: 10
}
}, [0]).then(function() {
expect(gd._fullData[0].transforms).toEqual([{
expect(gd._fullData[0].transforms).toEqual([jasmine.objectContaining({
enabled: true,
type: 'filter',
operation: '<',
filtersrc: 'x',
target: 'x',
value: 10
}]);
})]);

return Plots.transition(gd, [{
'transforms[0].operation': '>'
@@ -90,13 +90,13 @@ function runTests(transitionDuration) {
{duration: transitionDuration, easing: 'cubic-in-out'}
);
}).then(function() {
expect(gd._fullData[0].transforms).toEqual([{
expect(gd._fullData[0].transforms).toEqual([jasmine.objectContaining({
enabled: true,
type: 'filter',
operation: '>',
filtersrc: 'x',
target: 'x',
value: 10
}]);
})]);
}).catch(fail).then(done);
});