Skip to content

Bug null data #286

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
Feb 26, 2016
Merged

Bug null data #286

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b5309ec
Add test for connectgaps
mdtusz Feb 25, 2016
f3b679f
Add check for null or undefined points
mdtusz Feb 25, 2016
1c4a25d
fdescribe -> describe
mdtusz Feb 25, 2016
5ade8a8
Revert to original return statement
mdtusz Feb 25, 2016
a04550f
add null category image test mock
etpinard Feb 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: 6 additions & 3 deletions src/plots/cartesian/set_convert.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,10 +184,13 @@ module.exports = function setConvert(ax) {
// that aren't in the first etc.
// TODO: sorting options - do the sorting
// progressively here as we insert?
if(ax._categories.indexOf(v)===-1) ax._categories.push(v);

var c = ax._categories.indexOf(v);
return c===-1 ? constants.BADNUM : c;
if(v !== null && v !== undefined && ax._categories.indexOf(v) === -1){
Copy link
Contributor

@etpinard etpinard Feb 25, 2016

Choose a reason for hiding this comment

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

nice, maybe we should make that standard: http://eslint.org/docs/rules/no-eq-null

ax._categories.push(v);
return ax._categories.length - 1;
Copy link
Contributor

@etpinard etpinard Feb 25, 2016

Choose a reason for hiding this comment

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

Oh. I don't think this is right.

Because different traces can share categories, so the index of the incoming category here won't necessarily be the length - 1.

Copy link
Contributor Author

@mdtusz mdtusz Feb 25, 2016

Choose a reason for hiding this comment

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

Ah good catch. I suppose that's kinda the whole point of categories...

}else{
return constants.BADNUM;
}
};

ax.d2l = ax.d2c;
Expand Down
31 changes: 31 additions & 0 deletions test/jasmine/tests/calcdata_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
var Plotly = require('@lib/index');

var createGraphDiv = require('../assets/create_graph_div');
var destroyGraphDiv = require('../assets/destroy_graph_div');

describe('calculated data and points', function() {
describe('connectGaps', function() {

var gd;

beforeEach(function() {
gd = createGraphDiv();
});

afterEach(destroyGraphDiv);

it('should exclude null and undefined points when false', function() {
Plotly.plot(gd, [{ x: [1,2,3,undefined,5], y: [1,null,3,4,5]}], {});

expect(gd.calcdata[0][1]).toEqual({ x: false, y: false});
expect(gd.calcdata[0][3]).toEqual({ x: false, y: false});
});

it('should exclude null and undefined points as categories when false', function() {
Plotly.plot(gd, [{ x: [1,2,3,undefined,5], y: [1,null,3,4,5] }], { xaxis: { type: 'category' }});

expect(gd.calcdata[0][1]).toEqual({ x: false, y: false});
expect(gd.calcdata[0][3]).toEqual({ x: false, y: false});
Copy link
Contributor

@etpinard etpinard Feb 25, 2016

Choose a reason for hiding this comment

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

yeah man 🎉

});
});
});