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

distance keyword for data_labels has no effect in pie charts #183

Closed
JulienBacquart opened this issue Jul 1, 2024 · 4 comments · Fixed by #189
Closed

distance keyword for data_labels has no effect in pie charts #183

JulienBacquart opened this issue Jul 1, 2024 · 4 comments · Fixed by #189
Assignees
Labels
bug Something isn't working
Milestone

Comments

@JulienBacquart
Copy link
Contributor

Hi @hcpchris,

From the donut-charts.ipynb demos:

from highcharts_core.chart import Chart
from highcharts_core.options import HighchartsOptions

options_as_str = """{
    chart: {
        plotBackgroundColor: null,
        plotBorderWidth: 0,
        plotShadow: false
    },
    title: {
        text: 'Browser<br>shares<br>January<br>2022',
        align: 'center',
        verticalAlign: 'middle',
        y: 100
    },
    tooltip: {
        pointFormat: '{series.name}: <b>{point.percentage:.1f}%</b>'
    },
    accessibility: {
        point: {
            valueSuffix: '%'
        }
    },
    plotOptions: {
        pie: {
            dataLabels: {
                enabled: true,
                distance: -100,
                style: {
                    fontWeight: 'bold',
                    color: 'white'
                }
            },
            center: ['50%', '75%']
        }
    },
    series: [{
        type: 'pie',
        name: 'Browser share',
        innerSize: '50%',
        data: [
            ['Chrome', 73.86],
            ['Edge', 11.97],
            ['Firefox', 5.52],
            ['Safari', 2.98],
            ['Internet Explorer', 1.90],
            {
                name: 'Other',
                y: 3.77,
                dataLabels: {
                    enabled: false
                }
            }
        ]
    }]
}"""

options = HighchartsOptions.from_js_literal(options_as_str)
donut_chart = Chart.from_options(options)
donut_chart.display()

The distance keyword for the dataLabels has no effect, whatever the value.

Running:

donut_chart.to_json()

Return:

{
    "userOptions": {
        "accessibility": {"point": {"valueSuffix": "%"}},
        "chart": {
            "plotBackgroundColor": null,
            "plotBorderWidth": 0,
            "plotShadow": false,
        },
        "plotOptions": {
            "pie": {
                "center": ["50%", "75%"],
                "dataLabels": {
                    "enabled": true,
                    "style": {"fontWeight": "bold", "color": "white"},
                },
                "type": "pie",
            }
        },
        "series": [
            {
                "data": [
                    {"y": 73.86, "name": "Chrome"},
                    {"y": 11.97, "name": "Edge"},
                    {"y": 5.52, "name": "Firefox"},
                    {"y": 2.98, "name": "Safari"},
                    {"y": 1.9, "name": "Internet Explorer"},
                    {"y": 3.77, "dataLabels": {"enabled": false}, "name": "Other"},
                ],
                "name": "Browser share",
                "innerSize": "50%",
                "type": "pie",
            }
        ],
        "title": {
            "align": "center",
            "text": "Browser<br>shares<br>January<br>2022",
            "verticalAlign": "middle",
            "y": 100,
        },
        "tooltip": {"pointFormat": "{series.name}: <b>{point.percentage:.1f}%</b>"},
    }
}

Where the distance keyword is not present.

If I look at the data_labels class, I can't see no distance keyword.
But the JS class seems to have it: https://api.highcharts.com/highcharts/series.pie.dataLabels.distance

Trying to run this JS demo for the pie chart

options_as_str = """{
    chart: {
        type: 'pie'
    },
    title: {
        text: 'Egg Yolk Composition'
    },
    tooltip: {
        valueSuffix: '%'
    },
    subtitle: {
        text:
        'Source:<a href="https://www.mdpi.com/2072-6643/11/3/684/htm" target="_default">MDPI</a>'
    },
    plotOptions: {
        series: {
            allowPointSelect: true,
            cursor: 'pointer',
            dataLabels: [{
                enabled: true,
                distance: 20
            }, {
                enabled: true,
                distance: -40,
                format: '{point.percentage:.1f}%',
                style: {
                    fontSize: '1.2em',
                    textOutline: 'none',
                    opacity: 0.7
                },
                filter: {
                    operator: '>',
                    property: 'percentage',
                    value: 10
                }
            }]
        }
    },
    series: [
        {
            name: 'Percentage',
            colorByPoint: true,
            data: [
                {
                    name: 'Water',
                    y: 55.02
                },
                {
                    name: 'Fat',
                    sliced: true,
                    selected: true,
                    y: 26.71
                },
                {
                    name: 'Carbohydrates',
                    y: 1.09
                },
                {
                    name: 'Protein',
                    y: 15.5
                },
                {
                    name: 'Ash',
                    y: 1.68
                }
            ]
        }
    ]
}"""

options = HighchartsOptions.from_js_literal(options_as_str)
donut_chart = Chart.from_options(options)
donut_chart.display()

Result in all labels overlapping because of the distance keyword missing:
pie_chart_higharts_distance

@JulienBacquart JulienBacquart added the bug Something isn't working label Jul 1, 2024
@hcpchris hcpchris added this to the v.1.8.3 milestone Jul 2, 2024
@hcpchris
Copy link
Collaborator

hcpchris commented Jul 2, 2024

Thanks, @JulienBacquart : This looks to be a bug. I'll look into it and resolve it in the next release.

@JulienBacquart JulienBacquart changed the title Distance keyword for data_labels has no effect in pie charts distance keyword for data_labels has no effect in pie charts Jul 3, 2024
This was referenced Jul 16, 2024
@JulienBacquart
Copy link
Contributor Author

Hi @hcpchris

Looking at the code code, I don't think it will work, because of this part:

    def distance(self, value):
        if not value:
            self._distance = None

According to the PEP8 :

Also, beware of writing if x when you really mean if x is not None – e.g. when testing whether a variable or argument that defaults to None was set to some other value. The other value might have a type (such as a container) that could be false in a boolean context!

I think you should be using:

if value is None:
    self._distance = None

Because if you use:

if not value:
    self._distance = None

You will return None for all cases where value is equal to [], {}, (), '', 0, False.

The problem is 0 is a correct value for distance.
The default value is 30 : https://api.highcharts.com/highcharts/plotOptions.pie.dataLabels.distance
So setting distance to 0 has an effect, as you can see here: https://jsfiddle.net/1s972jkr/

I think I run in the same problem in other places:

    'tooltip': {
        'headerFormat': '', 
        ...,

In my opinion, the empty string should be a valid value to override the default headerFormat, but this code does nothing.
You have to do something like:

    'tooltip': {
        'headerFormat': 'null', 
        ...,

@hcpchris
Copy link
Collaborator

Thanks, @JulienBacquart : That's a good catch. Typically, if a false-y value is a valid value I employ the if value is None pattern, but in this case I failed to consider a distance value of 0. I'll patch that in a release of v.1.9.2 later today.

As for the tooltip.headerFormat, I'll take a closer look at that case and consider it in this release as well. There may be places where we're being overly broad in searching for False-y values, and other cases where we're not being broad enough.

@JulienBacquart
Copy link
Contributor Author

Hi @hcpchris ,
The first example works fine now, but the second example doesn't.

It's probably because we have different hierarchies, as a parameter for a pie or a series:

plotOptions: {
        pie: {
            dataLabels: {
                distance: -100,
plotOptions: {
        series: {
            dataLabels: [{
                distance: 20,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants