Sunburst chart calculate radius not counted exists of root area and inner radius

[moononournation]

Sunburst chart use d3.partition() to calculate the data size. This function reserved first level as root, but this area should not show in sunburst chart. Currently, second level shared this area, so the second level always larger that other level if inner radius is 0. In contrast, if setting inner radius too large, the first few levels cannot show.

So I suggest change the partition size setting to:

        var partition = d3.partition()
            // .size([2 * Math.PI, _radius * _radius]);
            .size([2 * Math.PI, 1]);

        partition(hierarchy);
        _rootOffset = hierarchy.y1;

And then change radius calculation in buildArcs() to:

            .innerRadius(function (d) {
                // return d.data.path && d.data.path.length === 1 ? _innerRadius : Math.sqrt(d.y0);
                return _innerRadius + ((d.y0 - _rootOffset) / (1 - _rootOffset) * (_radius - _innerRadius));
            })
            .outerRadius(function (d) {
                // return Math.sqrt(d.y1);
                return _innerRadius + ((d.y1 - _rootOffset) / (1 - _rootOffset) * (_radius - _innerRadius));
            });

[the3ver]

Thanks @moononournation your fix is working.

This behavior is actually an issue for us. The arcs of the actual values are beeing "eaten up" by the white inner radius. Or in other words, the radiusses of the value arcs do not adopt to the inner radius (root). Here is a minimal js fiddle : https://jsfiddle.net/the3ver/j1qedybp/14/ (dc.js 3.1.9): Changing the inner radius does not affect the distance of the outer ring from the center. If you set inner radius to 0 the additional space is not distributed to the rings.

We are using version 3.x. unfortuanetly and would need a fix there.
Here is a fiddle with dc.js 4, though: https://jsfiddle.net/the3ver/6ysokq1z/6/

[the3ver]

Inner raduis 0:

Inner radius 50:

Inner radius 150:

[gordonwoodhull]

Hi @the3ver, @moononournation. I did look at this when @moononournation filed it (and thought I commented on it, but maybe I got confused and didn't commit my comment).

Whenever changing the behavior I like to know:

1. What was the intent of the faulty code?
2. Will fixing it break other people's apps? Is it necessary to have an option to get back the old behavior?
3. Is there any way to write automated tests ?

TBH I'm still rather confused about this. It looks like there are a couple of strategies for how to use a sunburst, and the current code is using the same strategy as this block. However, this tutorial might be closer to the @moononournation solution (?)

Just a guess, but it seems like the answers are

1. It smells like the current code is trying to defeat d3.partition and not let it calculate the radii. The square in the partition initialization and then square root in the arc accessors are suspicious to me. @moononournation's solution is simpler and easier to understand. On the other hand, the d3.partition code is very simple and doesn't contain any squares or square roots that I could find, so this theory may be incorrect.
2. Unfortunately, probably changing it will break other people's apps.
3. Maybe.

Probably the quickest path forward, which wouldn't require any deeper understanding, is to add an option to the chart that allows choosing either behavior. If anyone wants to contribute a PR in that direction (defaulting to the old behavior), it would be easy to merge without worrying about breakage.

Thank you.

[alexnb]

Hi @gordonwoodhull, we found a solution for that, which even allows custom "relative ring sizes" and is still quite a small change. We'll post a corresponding pull request soon, hopefully with tests.

[the3ver]

Hi @gordonwoodhull,

since our project is still using 3.x, would you rather have a pull request based on the support/3.x branch first and after all is sorted and merged another one to port the changes up to 4? Or the other way round?

Cheers!

[gordonwoodhull]

Thanks @the3ver, @alexnb.

I am planning to maintain 3.x for a little while longer. A minor change like this should be easy to port from 3 to 4.

[gordonwoodhull]

Fixed by #1655 in 3.2.0 / 4.0.2