Add percentageDecimals options to pie chart, and use half even rounding

[thedustin]

📑 Summary

Rounding percentages is hard, as rounded percentages like to not sum up to 100% anymore.

This PR introduced two smaller changes, the use of the half even rounding mode, and a pie chart option to specify the number of digits to show in the chart. Both can be used to reduce or even fix greater or smaller than 100% issues.

Resolves #3987

📏 Design Decisions

Rounding Mode

There are different ways of rounding a number, the default way in JavaScript is the rounding half up mode (see mdn), which has a rounding bias and leads for percentages to sums over 100%. By using the rounding half even mode (also called "bankers' rounding", or "statistician's rounding"), we instead round .5 numbers to the nearest, even number. This rounding mode is to prefer, as it minimizes the overall error, and reduces rounding bias compared to other rounding modes like half up.

As you can see, this still does not make perfect 100%, but will lead to less than 100% instead of greater than 100%. I think this is to prefer, as a sum of greater than 100% always looks more suspicious than the other way around. Also, the question is if we should make this configurable too.

Different behaviour of rounding modes

percentageDecimals option

Another way of decreasing percentage sum errors is to less round the numbers, by using/showing more decimal places. At the moment the pie chart does not show decimals at all. I introduced an option percentageDecimals to allow setting the number of decimal places showed in the chart.

As the Intl.NumberFormat component already knows how to use different rounding modes, how to select the number of decimal places, and how to format percentages, I decided to use it here.

The allowed values are taken from the Intl.NumberFormat minimumFractionDigits options.

Pie Chart with decimal places

📋 Tasks

Make sure you

• 📖 have read the contribution guidelines
• 💻 have added necessary unit/e2e tests.
• 📓 have added documentation. Make sure MERMAID_RELEASE_VERSION is used for all new features.
• 🔖 targeted develop branch

[netlify]

❌ Deploy Preview for mermaid-js failed.

Name	Link
🔨 Latest commit	41a3ca2
🔍 Latest deploy log	https://app.netlify.com/sites/mermaid-js/deploys/660ef591d72258000829bac1

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b043d79) 73.73% compared to head (19deb5a) 85.10%.
Report is 2 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff              @@
##           develop    #5270       +/-   ##
============================================
+ Coverage    73.73%   85.10%   +11.37%     
============================================
  Files          164      174       +10     
  Lines        13864    11670     -2194     
  Branches       741      845      +104     
============================================
- Hits         10222     9932      -290     
+ Misses        3467     1542     -1925     
- Partials       175      196       +21     

Flag	Coverage Δ
e2e	85.10% <100.00%> (+6.95%)	⬆️
unit	?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
packages/mermaid/src/diagrams/pie/pieRenderer.ts	98.41% <100.00%> (+0.10%)	⬆️

... and 58 files with indirect coverage changes

[sidharthv96]

Awesome!
Love the PR, great code, description, tests and docs.

[aloisklink]

This looks like an amazing PR to me! I love the new Intl.NumberFormat, and the great tests and documentation!

The only issue I have is the custom rounding we're doing if pieConfig.percentageDecimals === 0.

I don't think we should do this, since it's a bit confusing to see 4.51% to be rounded to 4%, when you'd expect it to be rounded to 5%, even if you know about half-even rounding.

FYI, I don't think all browsers support the roundingMode: 'halfEven', since it's a pretty new feature, but that's fine, it will just fallback to the current default rounding behaviour 😄.

[aloisklink]

I'm not really sure this block is a good idea, since diagrams like below will change.

E.g., if I have 4.51%, I'd expect it to round to 5%, not to 4%.

```mermaid
pie title Pets adopted by volunteers
    "Dogs" : 5.51 %% should round up to 6
    "Cats" : 5.49 %% should round down to 5
    "Rats" : 4.51 %% should round up to 5
    "Mice" : 4.49 %% should round down to 4
    "Other": 80
```

pie title Pets adopted by volunteers
    "Dogs" : 5.51 %% should round up to 6
    "Cats" : 5.49 %% should round down to 5
    "Rats" : 4.51 %% should round up to 5
    "Mice" : 4.49 %% should round down to 4
    "Other": 80

[knsv]

I would agree with this

[sidharthv96]

@thedustin how can we fix this?

[thedustin]

I'm not sure about this. I think it's not fixable, as this merge requests explicitly introduced a new (more uncommon, but technically cleaner) way to round values. In this example you are right, you would notice the difference immediately (as you specified percentage values).

My goal was to find a way to resolve the "percentages add up to more than 100%" issue (#3987), but I agree with you that this way of rounding will confuse people.

As rounding always introduces some kind of error, maybe the better solution would be to keep the rounding as it is, and only introducing the decimal digits option. This way you can show the more precise values, when you need to prevent sum over 100%.