A strange interaction with crossfilter - subtle issue in Bubble Overlay

[kum-deepak]

Edit: now test case has been updated, so easier to review and merge.

Most charts uses d3.data, however, Bubble Overlay uses d3.datum.

There are two tests to check if filtering another dimension sets few circles to zero radii:

• https://github.com/dc-js/dc.js/blob/develop/spec/bubble-overlay-spec.js#L129
• https://github.com/dc-js/dc.js/blob/develop/spec/bubble-overlay-spec.js#L142

The test cases pass - but due to a strange coincidence. To demonstrate the issue I have made a change in the test cases which should not have made any difference (ae21b7a). However, with this change the test cases fail.

Underlying issue appears to be the following:

• When we bind using .datum, children inherit it.
• When we update .datum of the parent node, it reflects in the children.
• However, if we set .datum to undefined in the parent node, the children retain the older value.
• The fix is simple - instead of setting .datum to undefined we should set it to {} (commit 1759e22).

The unmodified code ties the crossfilter objects directly. When the filter is applied the tied objects get updated. This combined with the d3 behavior with undefined, makes the test cases succeed.

Not sure since which release the underlying issue appeared - it might be a d3 behavior change in d3v4.

I realize it is lot of explaining for a single line change.

[kum-deepak]

Even though work around is easy, do you think it may be a bug in d3?

[gordonwoodhull]

I think it's the documented behavior, but the author probably didn't think too much about reference semantics. There is a lot of stuff which works by accident in dc.js.

In this case, the object is assigned with .datum() when the <g> is created. Then it gets replicated downward by .append():

Each new element inherits the data of the current elements, if any, in the same manner as selection.select.

Since everything in JS is by reference, and group.all() returns the internal data structure, the <g> and its children all refer to the same value object, which gets updated by later crossfilter reductions. We depend on this all over the place.

The bubble overlay could have used .data() for the same effect; this would look like

this._g.data([data[point.name]]).append('g')

That's as far as I've gotten. I'd like to set a breakpoint on the test lines you pointed out, to understand what you're talking about, since I'm surprised this is needed.

[gordonwoodhull]

Do all the other charts allow cloning the crossfilter data? This surprises me, because I thought there were a lot of places that rely on the data being live-updated from crossfilter.

I think the actual bug here, if any, is that the bubble overlay chart is initialized with a particular set of points (California etc.) but the test uses removeEmptyBins(), which was probably a simple mistake. The bubble overlay is not designed to be used with removeEmptyBins; it expects to always have the same data coming in, with one crossfilter bin for every point.

The bubble overlay chart doesn't do any data joins; it doesn't have a way to add or remove points. Your fix causes it to substitute an empty object for any bins that have gone missing, and then presumably it coerces an undefined radius to zero. That make the test work but it's not really correct since the chart isn't designed to be used this way.

Would be happy to take a PR removing removeEmptyBins from the test.

[kum-deepak]

This is a fair point that this chart - the data should always include keys corresponding to each point. Should I change the test cases to not use removeEmptyBins()?

I have been trying various things with the code. One of them is to use without crossfilter. In our applications we routinely use simple charts, linked charts with cross filter, and linked charts where data is remote. Currently we use different libraries for each. Charts from these three libraries look and behave noticeably different. We are now starting to use dc for standalone charts as well. We achieve this by passing a fake group (with data from the backend) and a fake dimension.

On another note, cloning the cross filter data does not impact any other chart (barring Sunburst that has not been tested yet) - all test cases pass, examples seem to work. So, other than this, I have not come across any case that rely on bound objects getting modified by crossfilter. Are you aware of any?

[gordonwoodhull]

Yeah, I think the problem with this test is the use of removeEmptyBins. The chart always loops over points, so it doesn't make sense to remove group data.

That's a good point that data from other backends will usually not be modified in place the way crossfilter data is. Maybe I am remembering old issues that have been resolved by now.

I completely agree this should be supported!

What do you mean that charts look and behave noticeably different? Does it indicate bugs in dc.js?

[kum-deepak]

Yeah, I think the problem with this test is the use of removeEmptyBins. The chart always loops over points, so it doesn't make sense to remove group data.

I will do a PR

That's a good point that data from other backends will usually not be modified in place the way crossfilter data is. Maybe I am remembering old issues that have been resolved by now.

I completely agree this should be supported!

What do you mean that charts look and behave noticeably different? Does it indicate bugs in dc.js?

Because we did not use dc.js in other two cases 😄