Skip to content
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

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

Merged
merged 5 commits into from
Apr 26, 2017

Conversation

etpinard
Copy link
Contributor

peek 2017-04-12 15-11

@etpinard
Copy link
Contributor Author

@bpostlethwaite this resolves two feature requests of yours from last December.

@bpostlethwaite
Copy link
Member

nice! Will be interesting to integrate this back into Workspace

@@ -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?

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

'Determines whether or not gaps in data arrays produced by the filter operation',
'are preserved 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!

@@ -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!

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 🚀

- add COMPARISON_OPS list
- fixup `preservegaps` description
- add `preservegaps` commute tests
expect(out1[0].x).toEqual([undefined, undefined, undefined, 0, 1]);
expect(out1[0].y).toEqual([undefined, undefined, undefined, 1, 2]);
expect(out1[0].marker.color).toEqual([undefined, undefined, undefined, 0.1, 0.2]);
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

Awesome, thanks for those tests.

@alexcjohnson
Copy link
Collaborator

💃

@etpinard etpinard added this to the 1.27.0 milestone Apr 18, 2017
@etpinard etpinard merged commit 33196da into master Apr 26, 2017
@etpinard etpinard deleted the filter-preservegaps branch April 26, 2017 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants