Skip to content

Filter transform '!=' operation and 'preservegaps' option #1589

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.

Sign up for GitHub

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 5 commits into from
Apr 26, 2017
Merged

Filter transform '!=' operation and 'preservegaps' option #1589

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
52e9e4d
resolves #1177 - add '!=' filter operation
etpinard Apr 12, 2017
3f15c73
resolves #1178 - add preservegaps filter options
etpinard Apr 12, 2017
bb34df2
Merge branch 'master' into filter-preservegaps
etpinard Apr 18, 2017
1e0fe2f
AJ-proof filter preservegaps
etpinard Apr 18, 2017
2146bdd
merge INEQUALITY_OPS with COMPARISON_OPS
etpinard Apr 18, 2017
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
78 changes: 52 additions & 26 deletions src/transforms/filter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ 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 INEQUALITY_OPS = ['=', '!=', '<', '>=', '>', '<='];
Copy link
Collaborator

Choose a reason for hiding this comment

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

hmm, nonblocking as this isn't new, but seems funny to categorize '=' in INEQUALITY_OPS. How about COMPARISON_OPS?

var INTERVAL_OPS = ['[]', '()', '[)', '(]', '][', ')(', '](', ')['];
var SET_OPS = ['{}', '}{'];

Expand Down Expand Up @@ -57,6 +57,7 @@ exports.attributes = {
'Sets the filter operation.',

'*=* keeps items equal to `value`',
'*!=* keeps items not equal to `value`',

'*<* keeps items less than `value`',
'*<=* keeps items less than or equal to `value`',
Expand Down Expand Up @@ -101,7 +102,17 @@ exports.attributes = {
'*value* is expected to be an array with as many items as',
'the desired set elements.'
].join(' ')
}
},
preservegaps: {
valType: 'boolean',
dflt: false,
description: [
'Determines whether or not gaps in data arrays produced by the filter operation',
'are preserved or not.',
Copy link
Collaborator

Choose a reason for hiding this comment

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

whether or not... or not

'Setting this to *true* might be useful when plotting a line chart',
'with `connectgaps` set to *true*.'
Copy link
Collaborator

Choose a reason for hiding this comment

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

wait, do you mean with connectgaps set to false? connectgaps: true would run connect right over the gaps you just preserved...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh right. Good eye. Thanks!

].join(' ')
},
};

exports.supplyDefaults = function(transformIn) {
Expand All @@ -114,6 +125,7 @@ exports.supplyDefaults = function(transformIn) {
var enabled = coerce('enabled');

if(enabled) {
coerce('preservegaps');
coerce('operation');
coerce('value');
coerce('target');
Expand Down Expand Up @@ -149,36 +161,47 @@ exports.calcTransform = function(gd, trace, opts) {
var d2cTarget = (target === 'x' || target === 'y' || target === 'z') ?
target : filterArray;

var dataToCoord = getDataToCoordFunc(gd, trace, d2cTarget),
filterFunc = getFilterFunc(opts, dataToCoord, targetCalendar),
arrayAttrs = PlotSchema.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);
var dataToCoord = getDataToCoordFunc(gd, trace, d2cTarget);
var filterFunc = getFilterFunc(opts, dataToCoord, targetCalendar);
var arrayAttrs = PlotSchema.findArrayAttributes(trace);
var originalArrays = {};

originalArrays[attr] = Lib.extendDeep([], np.get());
np.set([]);
function forAllAttrs(fn, index) {
for(var j = 0; j < arrayAttrs.length; j++) {
var np = Lib.nestedProperty(trace, arrayAttrs[j]);
fn(np, index);
}
}

function fill(attr, i) {
var oldArr = originalArrays[attr],
newArr = Lib.nestedProperty(trace, attr).get();

newArr.push(oldArr[i]);
var initFn;
var fillFn;
if(opts.preservegaps) {
initFn = function(np) {
originalArrays[np.astr] = Lib.extendDeep([], np.get());
np.set(new Array(len));
};
fillFn = function(np, index) {
var val = originalArrays[np.astr][index];
np.get()[index] = val;
};
} else {
initFn = function(np) {
originalArrays[np.astr] = Lib.extendDeep([], np.get());
np.set([]);
};
fillFn = function(np, index) {
var val = originalArrays[np.astr][index];
np.get().push(val);
};
}

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

if(!filterFunc(v)) continue;
// copy all original array attribute values, and clear arrays in trace
forAllAttrs(initFn);

for(var j = 0; j < arrayAttrs.length; j++) {
fill(arrayAttrs[j], i);
}
// loop through filter array, fill trace arrays if passed
for(var i = 0; i < len; i++) {
var passed = filterFunc(filterArray[i]);
if(passed) forAllAttrs(fillFn, i);
}
};

Expand Down Expand Up @@ -262,6 +285,9 @@ function getFilterFunc(opts, d2c, targetCalendar) {
case '=':
return function(v) { return d2cTarget(v) === coercedValue; };

case '!=':
return function(v) { return d2cTarget(v) !== coercedValue; };

case '<':
return function(v) { return d2cTarget(v) < coercedValue; };

Expand Down
34 changes: 34 additions & 0 deletions test/jasmine/tests/transform_filter_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ describe('filter transforms defaults:', function() {
expect(traceOut.transforms).toEqual([{
type: 'filter',
enabled: true,
preservegaps: false,
operation: '=',
value: 0,
target: 'x',
Expand Down Expand Up @@ -320,6 +321,22 @@ describe('filter transforms calc:', function() {
expect(out[0].y).toEqual([1]);
});

it('should preserve gaps in data when `preservegaps` is turned on', function() {
var out = _transform([Lib.extendDeep({}, base, {
transforms: [{
type: 'filter',
preservegaps: true,
operation: '>',
value: 0,
target: 'x'
}]
})]);

expect(out[0].x).toEqual([undefined, undefined, undefined, undefined, 1, 2, 3]);
expect(out[0].y).toEqual([undefined, undefined, undefined, undefined, 2, 3, 1]);
expect(out[0].marker.color).toEqual([undefined, undefined, undefined, undefined, 0.2, 0.3, 0.4]);
});

Copy link
Collaborator

Choose a reason for hiding this comment

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

Are there any interesting cases to test with two filters applied in sequence having different preservegaps values?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not yet. I'll add one in a future commit 🚀

describe('filters should handle numeric values', function() {
var _base = Lib.extendDeep({}, base);

Expand Down Expand Up @@ -595,6 +612,23 @@ describe('filter transforms calc:', function() {
_assert(out, ['2015-07-20'], [1], [0.1]);
});

it('with operation *!=*', function() {
var out = _transform([Lib.extendDeep({}, _base, {
transforms: [{
operation: '!=',
value: '2015-07-20',
target: '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('with operation *<*', function() {
var out = _transform([Lib.extendDeep({}, _base, {
transforms: [{
Expand Down
14 changes: 8 additions & 6 deletions test/jasmine/tests/transform_multi_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ describe('general transforms:', function() {
operation: '=',
value: 0,
target: 'x',
preservegaps: false,
_module: Filter
}]);
});
Expand Down Expand Up @@ -66,23 +67,23 @@ describe('general transforms:', function() {

traceOut = Plots.supplyTraceDefaults(traceIn, 0, layout);

expect(traceOut.transforms[0]).toEqual({
expect(traceOut.transforms[0]).toEqual(jasmine.objectContaining({
Copy link
Collaborator

Choose a reason for hiding this comment

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

oh, didn't know about that one, cool!

type: 'filter',
enabled: true,
operation: '=',
value: 0,
target: 'x',
_module: Filter
}, '- global first');
}), '- global first');

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

expect(layout._transformModules).toEqual([Filter]);
});
Expand Down Expand Up @@ -118,14 +119,14 @@ describe('general transforms:', function() {
}], msg);

msg = 'supplying the transform defaults';
expect(dataOut[1].transforms[0]).toEqual({
expect(dataOut[1].transforms[0]).toEqual(jasmine.objectContaining({
type: 'filter',
enabled: true,
operation: '>',
value: 0,
target: 'x',
_module: Filter
}, msg);
}), msg);

msg = 'keeping refs to user data';
expect(dataOut[1]._input.x).toEqual([-2, -1, -2, 0, 1, 2, 3], msg);
Expand All @@ -144,6 +145,7 @@ describe('general transforms:', function() {
operation: '>',
value: 0,
target: 'x',
preservegaps: false,
_module: Filter
}], msg);

Expand Down