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

Make pyplot Configurable #51

Open
EntilZha opened this issue Feb 28, 2020 · 2 comments
Open

Make pyplot Configurable #51

EntilZha opened this issue Feb 28, 2020 · 2 comments
Labels
enhancement New feature or request

Comments

@EntilZha
Copy link

EntilZha commented Feb 28, 2020

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
Copy link
Owner

apiad commented Feb 28, 2020

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
Copy link
Owner

apiad commented Feb 28, 2020 via email

@apiad apiad added the enhancement New feature or request label Feb 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants