apiad / auditorium

An HTML+CSS+JS generator from pure Python code
https://apiad.net/auditorium
81 stars 13 forks source link

Make pyplot Configurable #51

Open EntilZha opened 4 years ago

EntilZha commented 4 years ago

I've been giving a shot to auditorium for presentations and like it so far, but there are a few things that it would be great to make configurable. I'd love to submit a PR, but wanted to check on preferred approach to implementing feature.

The issue I'm trying to solve is that when using plotnine, many figures (especially legends) get cropped out. I traced this down to that pyplot https://github.com/apiad/auditorium/blob/master/auditorium/show.py#L340 calls plt.tight_layout(). On the other hand, plotnine does not call that, but makes a call like plt.savefig(filename, bbox_inches='tight' (https://plotnine.readthedocs.io/en/stable/_modules/plotnine/ggplot.html#ggplot.save).

I'd propose something like:

  1. Add a parameter to pyplot like savefig_args and change the savefig call to plt.savefig(buffer, format=fmt, **savefig_args)
  2. Add a parameter tight_layout=True (or False) so only call plt.tight_layout() if it is True

Combined, this would make it so that I can display plotnine plots as they're intended to look, without monkey patching the pyplot call. Another possibility is in addition to (1)/(2), add argument on Show where the defaults can be set, although a similar thing could be accomplished in user code like

def myplot(ctx, plt):
    ctx.pyplot(plt, savefig_args={}, tight_layout=False)

def myslide(ctx):
    myplot(ctx, someplot)
apiad commented 4 years ago

Hey, I'm really happy to hear that someone finds this project useful!

Of course, I would love to merge your requests, if it makes your use case easier. I like the approach of making everything configurable but with sensible defaults, so if tight_layout is something that most of the time would work better as True, then I would prefer to have that as an optional parameter with default value True. If I understand well this is what your proposal is actually about.

In any case, yes, feel free to make the changes you consider relevant and I'll be more than happy to merge your pull requests. Another thing, there are unmerged changes in develop so maybe it is best if you branch out from there instead of master because in the long run those changes will be merged in. If this creates too much trouble then don't worry, it's just so that later those changes are easier to merge.

Finally, it's not absolutely necessary, but if you feel that adding an example to the demo slide is useful, then be my guest. We unit-test the demo (at least that it runs without errors) so this at least ensures that the parameters used are in sync with the API.

apiad commented 4 years ago

Regarding your second point, right now in develop there should be a parameter to change the Context class in Show so you can inherit and override some methods cleanly outside any slide code.