JackMcKew / pandas_alive

Create stunning, animated visualisations with Pandas & Matplotlib as easy as calling `df.plot_animated()`
MIT License
581 stars 100 forks source link

period_summary_func parameter ignored on pie chart #20

Closed jrichdyer closed 3 years ago

jrichdyer commented 3 years ago

When calling plot_animated() and specifying kind='pie', the period_summary_func parameter appears to be ignored. I tried specifying my own figure and adding my own text with ax.text() but that fails to show up in the pie chart animation as well.

After a little digging, it looks like that's because the text is explicitly removed in charts.py, on lines 633-644:

         for text in self.ax.texts[int(bool(self.period_fmt)) :]:
             text.remove()

This can't be commented out, since it prevents wedge labels from piling up on top of each other as the wedges change. The only thing I could think of to fix it was to add the text created by period_summary_func back in afterward. To do that, I added the following code (which is just copied from lines 551-563 of _base_chart.py) to charts.py after line 650:

        if self.period_summary_func:
            values = self.df.iloc[i]
            text_dict = self.period_summary_func(values)
            if "x" not in text_dict or "y" not in text_dict or "s" not in text_dict:
                name = self.period_summary_func.__name__
                raise ValueError(
                    f"The dictionary returned from `{name}` must contain "
                    '"x", "y", and "s"'
                )
            if len(self.ax.texts) != 2:
                self.ax.text(transform=self.ax.transAxes, **text_dict)
            else:
                self.ax.texts[1].set_text(text_dict["s"])

This works as a quick fix for me, but it's not the most elegant solution, which is why I chose to throw it here instead of creating a PR.

JackMcKew commented 3 years ago

Hey there! Nice pickup & thank you for raising this πŸ˜„

Here's a reproducible example where we can note that the current version doesn't show the period_summary_func text:

import pandas_alive

covid_df = pandas_alive.load_dataset()

def current_total(values):
    total = values.sum()
    s = f"Total : {int(total)}"
    return {"x": 0.85, "y": 0.2, "s": s, "ha": "right", "size": 11}

covid_df.plot_animated(
    filename="test.gif",
    kind="pie",
    period_summary_func=current_total,
    enable_progress_bar=True,
)

It seems as if BarChartRace already had a fix for this which wasn't transferred to other chart types in that _base_chart.show_period(i) is called after plot_bars to ensure that the period_summary_func isn't lost.

I've implemented this fix for all other chart types as well. πŸ˜„ (and took the opportunity to do some refactoring as well). Find the changes at https://github.com/JackMcKew/pandas_alive/commit/1776c5a838e9e6bc4c36b0d82114c90c5ca3f929

After testing happens, I'll be sure to make a release but not sure on the timeline at this point in time.

JackMcKew commented 3 years ago

I'll close this when it gets released on PyPI

JackMcKew commented 3 years ago

0.2.4 is now released πŸ˜„