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

Exception: Filtering an Empty pie chart using an object-valued Crossfilter group causes an error #1085

Closed
XaserAcheron opened this issue Jan 8, 2016 · 2 comments
Labels
Milestone

Comments

@XaserAcheron
Copy link
Contributor

Hey folks,

Just ran into a nasty exception in a project of mine and managed to isolate it.

The short version is that for a dashboard I'm building, I have a need to aggregate more than one value in a crossfilter group (i.e. like reduceSum except the output is an object containing multiple values). I've written custom reduce functions to do this, and it works pretty well except for one particular case.

The nefarious bug: If a pie chart that references this group becomes empty due to filtering, an internal exception is thrown whenever the pie chart attempts to show data again. This breaks further script processing, effectively requiring a page refresh.

Here's a fiddle and an error screenshot:
https://jsfiddle.net/mojj3dcp/
http://i.imgur.com/JTsAVyo.png

Behind-the-scenes: dc.js's tweenPie function calls d3.interpolate on the "empty" slice (where data.value is a number) and a slice which contains data (where data.value is an object); d3.interpolate chokes on trying to interpolate the number and the object, effectively "crashing" dc.

I hackily fixed this in my local dc.js by adding a type check to tweenPie as follows:

function tweenPie (b) {
    b.innerRadius = _innerRadius;
    var current = this._current;
    if (isOffCanvas(current) || (typeof b.data.value !== typeof current.data.value)) { // [XA] hacky edit
        current = {startAngle: 0, endAngle: 0};
    }
    var i = d3.interpolate(current, b);
    this._current = i(0);
    return function (t) {
        return safeArc(i(t), 0, buildArcs());
    };
}

The check can probably be written a bit better, but it works.

Note that while I'm referencing the dc.js build from the github.io site (2.0.0-beta.25) in the fiddle, I wrote the test locally on the lastest master revision and it happens there too (i.e. bug still present).

Hope this makes sense, and many thanks for the super-cool tool,
-X

@gordonwoodhull
Copy link
Contributor

Thanks for the report, @XaserAcheron!

The code looks really screwy to me because it's interpolating the whole data object instead of just the {startAngle, endAngle} that it needs. I bet there are other ways to break this by changing the shape/type of the data.

I don't think we have a test restoring the chart for the value accessor case. I think it belongs here but I wasn't able to reproduce the problem with a quick try. Love your example, could I use it as a Jasmine test case if I can't get the existing test to work?

@gordonwoodhull gordonwoodhull added this to the v2.0 milestone Jan 11, 2016
@XaserAcheron
Copy link
Contributor Author

re: using the code -- sure! Glad to hear it's useful.

I'm a bit surprised too that d3.interpolateObject doesn't check for this case, but it's not like my own code makes any less assumptions. :P

[EDIT] Per your observation, I quick-hacked in added an 'else' clause that drops all the properties from current except for startAngle/endAngle and it works just fine:

function tweenPie (b) {
    b.innerRadius = _innerRadius;
    var current = this._current;
    if (isOffCanvas(current)) {
        current = {startAngle: 0, endAngle: 0};
    } else { // [XA] only interpolate statAngle & endAngle, not the whole data object
        current = {startAngle: current.startAngle, endAngle: current.endAngle};
    }
    var i = d3.interpolate(current, b);
    this._current = i(0);
    return function (t) {
        return safeArc(i(t), 0, buildArcs());
    };
}

Better than the silly type check I put in earlier, and it ought to be future-proof.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants