Skip to content

[WIP] Transforms [rebase of timelyportfolio's work] #936

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 34 commits into from
Sep 26, 2016
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
9bb2b10
move `transforms` to a "proper" place
timelyportfolio Aug 3, 2016
582d210
add `in` \ `notin` and `within` \ `notwithin` filter transforms; regi…
timelyportfolio Aug 3, 2016
d0ef359
use value array for array comparisons and camelCase
timelyportfolio Aug 4, 2016
9574f4f
accept '0' and ['0'] in filter
timelyportfolio Aug 12, 2016
77e2b27
ignore character values for `within` filter
timelyportfolio Aug 12, 2016
aa1f1d3
use `Lib.isDateTime`
timelyportfolio Aug 12, 2016
6015768
clean and lint
timelyportfolio Aug 17, 2016
6daeac5
remove filter register in validate_test
timelyportfolio Aug 17, 2016
3e0f209
try to get header correct
timelyportfolio Aug 17, 2016
d735b64
minor cleanup
monfera Sep 14, 2016
a820b0c
commenting out tests not yet passing
monfera Sep 14, 2016
2dd3646
require the transforms
monfera Sep 14, 2016
c30f94e
split large and growing test file to three
monfera Sep 16, 2016
e9bde33
groupby generalization
monfera Sep 16, 2016
c6355e8
switch from 'styles' to 'style'
monfera Sep 16, 2016
5765f66
groupby generalization - update test cases
monfera Sep 16, 2016
02742e8
adding ids to filtersrc
monfera Sep 16, 2016
b23d2ff
[spacing][technical] just wrapping around test case in anticipation o…
monfera Sep 16, 2016
55f9e0c
symmetry / degeneracy test cases (manually cherrypicked)
monfera Sep 16, 2016
462b2f0
test heterogeneous attributes, deep nesting and overridden attributes
monfera Sep 16, 2016
9cafe1e
groupby: don't mandate styling for each group, or any style at all
monfera Sep 16, 2016
87de31f
groupby test for no style present
monfera Sep 16, 2016
61f3385
hierarchical property split (squashed)
monfera Sep 21, 2016
57d0c13
reuse the plotly_schema crawler and fix some crawling issues (squashed)
monfera Sep 22, 2016
7085729
resolved circular dependency by extracting the crawler
monfera Sep 22, 2016
392844d
sync up filter and groupby; fix combined test case which used to pass…
monfera Sep 22, 2016
1dd93bb
PR feedback: comment and test case updates
monfera Sep 22, 2016
1ea2fd7
PR feedback: test with empty grouping information
monfera Sep 23, 2016
59b741d
PR feedback: moved isValObject into coerce
monfera Sep 23, 2016
a5dad2a
PR feedback: moved crawler and its constituent parts into coerce
monfera Sep 23, 2016
79dcbc3
Minor: var rename
monfera Sep 23, 2016
aa70e4c
PR feedback: don't split if `groups` is unsupplied, non-array or of z…
monfera Sep 23, 2016
523f61b
PR feedback: disuse in favor of Lib.nestedProperty getter/setter
monfera Sep 23, 2016
4efe6a9
PR feedback: pruning in-progress comments
monfera Sep 26, 2016
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
9 changes: 9 additions & 0 deletions lib/filter.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/**
* 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.
*/

module.exports = require('../src/transforms/filter');
5 changes: 5 additions & 0 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,9 @@ Plotly.register([
require('./scattermapbox')
]);

// add transforms
Plotly.register([
require('./filter')
]);

module.exports = Plotly;
Original file line number Diff line number Diff line change
@@ -1,11 +1,20 @@
/**
* 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';

var isNumeric = require('fast-isnumeric');

// var Lib = require('@src/lib');
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';

Expand All @@ -16,11 +25,11 @@ exports.name = 'filter';
exports.attributes = {
operation: {
valType: 'enumerated',
values: ['=', '<', '>'],
values: ['=', '<', '>', 'within', 'notwithin', 'in', 'notin'],
dflt: '='
},
value: {
valType: 'number',
valType: 'any',
dflt: 0
},
filtersrc: {
Copy link
Contributor

@rreusser rreusser Sep 14, 2016

Choose a reason for hiding this comment

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

Can we add ids (edit: not identifiers) to the filtersrc enum? The previous blocker was the animation PR getting merged, but now that it's merged, it's probably a good idea. Without that, I believe @timelyportfolio had to use floating point equaity in order to use the lasso selector and then filter to the selected set. Since in already uses indexOf, I think this perhaps really is a one-liner.

Copy link
Contributor

Choose a reason for hiding this comment

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

you mean ids ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you, yes. I was looking at the wrong branch. ids.

Expand Down Expand Up @@ -54,6 +63,16 @@ exports.supplyDefaults = function(transformIn, fullData, layout) {
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;
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer moving the array item type conversion down to the transform step - supplyDefaults (even the transform version) should try to avoid diving into arrays.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

FYI I haven't touched anything in filter.js so it's now as it is in @timelyportfolio's branch (and I haven't yet pushed the changes to groupby.js. If I understand the task correctly, I need to work on groupby as part of the first sprint on it so I'll get back on it in a bit.

}

// or some more complex logic using fullData and layout

return transformOut;
Expand Down Expand Up @@ -121,6 +140,16 @@ function transformOne(trace, state) {

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;
}

switch(opts.operation) {
case '=':
Expand All @@ -129,6 +158,30 @@ function getFilterFunc(opts) {
return function(v) { return v < value; };
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; };
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,15 @@
/**
* 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';

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

/* eslint no-unused-vars: 0*/

Expand Down
4 changes: 2 additions & 2 deletions test/jasmine/tests/plotschema_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ var Plotly = require('@lib/index');
var Lib = require('@src/lib');

Plotly.register([
// until they become official
require('../assets/transforms/filter')
require('@src/transforms/filter'),
require('@src/transforms/groupby')
]);

describe('plot schema', function() {
Expand Down
Loading