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.

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

Jump to bottom

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

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from
+60 −7
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
127b53f
Add percentageDecimals options to pie chart, and use half even roundi…
thedustin Feb 5, 2024
5f8e2b3
Add version placeholder to percentageDecimals option
thedustin Feb 5, 2024
af4c232
Limit percentageDecimals to 20, the maximum value of Intl.NumberForma…
thedustin Feb 5, 2024
8f3e6c4
Fix linting in config file
thedustin Feb 5, 2024
19deb5a
Generate docs/
thedustin Feb 5, 2024
41a3ca2
Update packages/mermaid/src/schemas/config.schema.yaml
thedustin Apr 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 22 additions & 0 deletions cypress/integration/rendering/pie.spec.ts
View file
Open in desktop
Expand Up @@ -74,6 +74,17 @@ describe('pie chart', () => {
);
});

it('should render a pie diagram when percentageDecimals is setted', () => {
imgSnapshotTest(
`pie
"Dogs" : 80
"Cats" : 86
"Rats" : 80
`,
{ logLevel: 1, pie: { percentageDecimals: 2 } }
);
});

it('should render a pie diagram with showData', () => {
imgSnapshotTest(
`pie showData
Expand All @@ -82,4 +93,15 @@ describe('pie chart', () => {
`
);
});

it('should render a pie diagram and use half even rounding by default', () => {
// depending on the rounding method, the sum of the percentages can be 101 (default rounding), or 99 (half even)
imgSnapshotTest(
`pie
"Dogs" : 80
"Cats" : 86
"Rats" : 80
`
);
});
});
7 changes: 4 additions & 3 deletions docs/syntax/pie.md
View file
Open in desktop
Expand Up @@ -71,6 +71,7 @@ pie showData

Possible pie diagram configuration parameters:

| Parameter | Description | Default value |
| -------------- | ------------------------------------------------------------------------------------------------------------ | ------------- |
| `textPosition` | The axial position of the pie slice labels, from 0.0 at the center to 1.0 at the outside edge of the circle. | `0.75` |
| Parameter | Description | Default value |
| -------------------- | ------------------------------------------------------------------------------------------------------------ | ------------- |
| `textPosition` | The axial position of the pie slice labels, from 0.0 at the center to 1.0 at the outside edge of the circle. | `0.75` |
| `percentageDecimals` | The number of decimal places to show in the percentage label, from 0 to 20. (v\<MERMAID_RELEASE_VERSION>+) | `0` |
5 changes: 5 additions & 0 deletions packages/mermaid/src/config.type.ts
View file
Open in desktop
Expand Up @@ -641,6 +641,11 @@ export interface PieDiagramConfig extends BaseDiagramConfig {
*
*/
textPosition?: number;
/**
* The number of decimal places to show in the percentage label.
*
*/
percentageDecimals?: number;
}
/**
* This interface was referenced by `MermaidConfig`'s JSON-Schema
Expand Down
3 changes: 3 additions & 0 deletions packages/mermaid/src/diagrams/pie/pie.spec.ts
View file
Open in desktop
Expand Up @@ -164,6 +164,9 @@ describe('pie', () => {
// db.setConfig({ textPosition: 0 });
// db.resetConfig();
expect(db.getConfig().textPosition).toStrictEqual(DEFAULT_PIE_DB.config.textPosition);
expect(db.getConfig().percentageDecimals).toStrictEqual(
DEFAULT_PIE_DB.config.percentageDecimals
);
});
});
});
16 changes: 15 additions & 1 deletion packages/mermaid/src/diagrams/pie/pieRenderer.ts
View file
Open in desktop
Expand Up @@ -50,6 +50,12 @@ export const draw: DrawDefinition = (text, id, _version, diagObj) => {
const sections: Sections = db.getSections();
group.attr('transform', 'translate(' + pieWidth / 2 + ',' + height / 2 + ')');

const percentFormatter = new Intl.NumberFormat('en-US', {
style: 'percent',
roundingMode: 'halfEven', // Half even rounding is used to avoid the sum of the percentages to be greater than 100%.
minimumFractionDigits: pieConfig.percentageDecimals,
} as Intl.NumberFormatOptions);

const { themeVariables } = globalConfig;
let [outerStrokeWidth] = parseFontSize(themeVariables.pieOuterStrokeWidth);
outerStrokeWidth ??= 2;
Expand Down Expand Up @@ -118,7 +124,15 @@ export const draw: DrawDefinition = (text, id, _version, diagObj) => {
.enter()
.append('text')
.text((datum: d3.PieArcDatum<D3Sections>): string => {
return ((datum.data.value / sum) * 100).toFixed(0) + '%';
let num = datum.data.value / sum;

// "Half even" rounding mode rounds exactly .5 to the nearest, even number.
// By using 3 decimal places, we can round "close to .5" to the nearest even number too.
if (pieConfig.percentageDecimals === 0) {
num = Number(num.toFixed(3));
}

Comment on lines +128 to +134
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

I would agree with this

Copy link
Member

Choose a reason for hiding this comment

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

@thedustin how can we fix this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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%.

return percentFormatter.format(num);
})
.attr('transform', (datum: d3.PieArcDatum<D3Sections>): string => {
return 'translate(' + labelArcGenerator.centroid(datum) + ')';
Expand Down
7 changes: 4 additions & 3 deletions packages/mermaid/src/docs/syntax/pie.md
View file
Open in desktop
Expand Up @@ -48,6 +48,7 @@ pie showData

Possible pie diagram configuration parameters:

| Parameter | Description | Default value |
| -------------- | ------------------------------------------------------------------------------------------------------------ | ------------- |
| `textPosition` | The axial position of the pie slice labels, from 0.0 at the center to 1.0 at the outside edge of the circle. | `0.75` |
| Parameter | Description | Default value |
| -------------------- | ------------------------------------------------------------------------------------------------------------ | ------------- |
| `textPosition` | The axial position of the pie slice labels, from 0.0 at the center to 1.0 at the outside edge of the circle. | `0.75` |
| `percentageDecimals` | The number of decimal places to show in the percentage label, from 0 to 20. (v<MERMAID_RELEASE_VERSION>+) | `0` |
7 changes: 7 additions & 0 deletions packages/mermaid/src/schemas/config.schema.yaml
View file
Open in desktop
Expand Up @@ -876,6 +876,13 @@ $defs: # JSON Schema definition (maybe we should move these to a separate file)
description: |
Axial position of slice's label from zero at the center to 1 at the outside edges.
default: 0.75
percentageDecimals:
type: integer
minimum: 0
maximum: 20
description: |
The number of decimal places to show in the percentage label.
default: 0

QuadrantChartConfig:
title: Quadrant Chart Config
Expand Down