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

add InlineBackend.print_figure_kwargs #5110

Merged
merged 10 commits into from Feb 21, 2014
Merged

Conversation

minrk
Copy link
Member

@minrk minrk commented Feb 12, 2014

removes special handling of 'quality', in favor of passing arbitrary kwargs to fig.canvas.print_figure.

This lets people override the occasionally problematic use of bbox_inches='tight', and any potential future issue that might be solved by adding/changing kwargs to print_figure.

@@ -10,8 +10,11 @@
import nose.tools as nt

from IPython.core import display
from IPython.core.getipython import get_ipython
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't actually needed - we inject get_ipython into builtins for the tests.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I always use the API, rather than relying on injection in library code. I see the injection as exclusively for interactive use.

@takluyver
Copy link
Member

Apart from inline nitpicking, looks good.

removes special handling of 'quality', in favor of passing arbitrary kwargs to fig.canvas.print_figure.

This lets people override the occasionally problematic use of `bbox_inches='tight'`,
and any potential future issue that might be solved by adding/changing kwargs to print_figure.
@@ -0,0 +1,3 @@
* added ``InlineBackend.print_figure_kwargs`` to allow passing keyword arguments
to matplotlib's ``Canvas.print_figure``. This can be used to change the value of
``bbox_inches``, which is 'tight' by default, or set the quality of JPEG figures.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not quite sure of the scope of our what's new doc, but maybe you should put a note that the InlineBackend.quality flag was removed? Then users can find in the docs why their figure output starts to behave in an unexpected manner because they didn't change their quality flag to use print_figure_kwargs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

quality wasn't in 1.x, though - I think the whatsnew doc only needs to point out the differences between stable releases. People running on master should be aware that APIs are liable to change.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah thanks for clarifying

@ellisonbg
Copy link
Member

A few inline comments to be addressed, but overall looks great, especially the new tests.

ellisonbg added a commit that referenced this pull request Feb 21, 2014
add InlineBackend.print_figure_kwargs
@ellisonbg ellisonbg merged commit 3d36519 into ipython:master Feb 21, 2014
@minrk minrk deleted the bbox_inches branch March 31, 2014 23:36
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
add InlineBackend.print_figure_kwargs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants