pymc-labs / CausalPy

A Python package for causal inference in quasi-experimental settings
https://causalpy.readthedocs.io
Apache License 2.0
829 stars 52 forks source link

Fix legend overlapping with lines on causal impact plots #341

Open drbenvincent opened 1 month ago

drbenvincent commented 1 month ago

In a number of situations, the legend is overlapping with data in the causal impact plots. Here's an example:

Screenshot 2024-05-08 at 21 06 46
anevolbap commented 1 month ago

Adding something like this should work: sns.move_legend(ax[0], "upper left", bbox_to_anchor=(1, 1))

Maybe figsize can be adjusted to make wider as before :thinking:

image

drbenvincent commented 3 weeks ago

Sorry for slow reply @anevolbap - notification got buried.

I think that could be good, though I'd prefer direct matplotlib manipulation rather than seaborn, I think it's still just a 1 liner.

I'm also wondering about placing the legend at the bottom. Maybe we can have a kwarg for the plot method where we can ask for "bottom" or "right" for example.

I can't remember now, but I believe we pass back the figure and axis handles, so if a user wanted fine-grained control then they could do that after the call to result.plot().

anevolbap commented 3 weeks ago

No worries, @drbenvincent. I'm quite busy over here too. I'll work on your comments. Thanks!

OriolAbril commented 2 weeks ago

Instead of using seaborn's move_legend (which actually deletes the legend and creates a new one under the hood), you can pass these same arguments to the call to ax.legend. Would the goal be to allow users to pass arbitrary kwargs to ax.legend or would it be preferrable to have a single legend kwarg that takes a few strings representing common placements?

drbenvincent commented 2 weeks ago

I don't have strong feelings about it, but my slight preference would be towards allowing the user more control. As long as we have an example (and info in the plot docstrings) then it shouldn't be too tricky for people to figure out.