Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Adding png image return for inline backend figures with _repr_html_#16788

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
tdpetrou wants to merge5 commits intomatplotlib:masterfromtdpetrou:add-repr_html-for-inline-figures
Closed

Adding png image return for inline backend figures with _repr_html_#16788

tdpetrou wants to merge5 commits intomatplotlib:masterfromtdpetrou:add-repr_html-for-inline-figures

Conversation

tdpetrou
Copy link
Contributor

@tdpetroutdpetrou commentedMar 16, 2020
edited
Loading

PR Summary

This will enable figures to be displayed in a jupyter notebook without ever having to run%matplotlib inline orimport matplotlib.pyplot.

The initial issue was created here#16782.

I'm sure the code will need to be modified quite a bit to work with more cases, but I wanted to get the ball started on this important issue for me.

I don't know how to test this. Are there special tests for jupyter notebook output?

PR Checklist

  • Has Pytest style unit tests
  • Code isFlake 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

philippjfr and mdeff reacted with thumbs up emojijbednar and philippjfr reacted with hooray emoji
@@ -375,6 +375,15 @@ def _repr_html_(self):
from matplotlib.backends import backend_webagg
return backend_webagg.ipython_inline_display(self)

if mpl.rcParams['backend'] == 'module://ipykernel.pylab.backend_inline':
Copy link
Contributor

Choose a reason for hiding this comment

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

this should (likely?) check the canvas type (as for webagg), because rcParams["backend"] may change?

tacaswell and story645 reacted with thumbs up emoji
Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

That makes sense - should we have a figure output for all backends? And just choose the image format based on that backend?

Copy link
Member

Choose a reason for hiding this comment

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

Instead of adding more coditional paths here we should make this defer to a_repr_html_ on the canvas.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

@tacaswell Would adding a single_repr_html_ tobackend_bases.FigureCanvaseBase be enough?

Copy link
Member

Choose a reason for hiding this comment

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

No, because for the interactive backends we do not want to replace them with a dead png.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

But this would only get triggered if a figure was the output of the last line of a cell in a notebook, which would (likely) only be done intentionally.

@story645
Copy link
Member

Awesome, thanks for opening this issue!

tacaswell
tacaswell previously requested changesMar 16, 2020
@@ -375,6 +375,15 @@ def _repr_html_(self):
from matplotlib.backends import backend_webagg
return backend_webagg.ipython_inline_display(self)

if mpl.rcParams['backend'] == 'module://ipykernel.pylab.backend_inline':
Copy link
Member

Choose a reason for hiding this comment

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

Instead of adding more coditional paths here we should make this defer to a_repr_html_ on the canvas.

@tdpetrou
Copy link
ContributorAuthor

I changed the approach completely and added a_repr_html_ tobackend_bases.FigureCanvasBase. It gets the default format type and returns that format as HTML. It defaults to png whenever the format is not one of png, svg, jpg, or pdf. I used the<embed> tag for pdfs.

In Figure, the_repr_html_ defers to the canvas_repr_html_ for all cases. I moved the existing code to thebackend_webagg.FigureCanvasWebAgg class under its own_repr_html_.


fmt = self.get_default_filetype()
dpi = self.figure.dpi
if fmt == 'retina':
Copy link
Contributor

Choose a reason for hiding this comment

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

is there really a backend that defines such a default filetype?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

FigureCanvasBase defaults torcParams['savefig.format']. svg, pdf, pgf, ps all have this method implemented.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, but the format won't be 'retina'.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Thank you for this. I see what I need to do. Get the ipython shell and check for retina as the figure_format. I would also need to get other settings like bbox_inches from there.

Regarding retina - it implicitly doubles the DPI, but this gives a false impression of what a saved image will look like. I think it would be better to change the rcparams. Even better would be to have a backend titled 'inline_real' and set the DPI to the DPI of the monitor (and adjust the sizes of all fonts, lines, tick lines, etc.. accordingly.).

Copy link
Contributor

Choose a reason for hiding this comment

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

I am somewhat familiar with the problems of retina (well, on a theoretical PoV). My point is just that get_default_filetype() never returns (AFAIK) "retina"; that info is simply stashed elsewhere.


mimetype_dict = {'png': 'image/png', 'svg': 'image/svg+xml',
'jpg': 'image/jpeg', 'pdf': 'application/pdf'}
if fmt not in mimetype_dict:
Copy link
Contributor

Choose a reason for hiding this comment

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

falling back to pnghere seems wrong/too late (it could well not be a png file, indeed).

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

As of now, running %matplotlib inline uses default figure_format as png. So regardless of the backend, a png will be produced.

}

bytes_io = io.BytesIO()
self.print_figure(bytes_io, **kw)
Copy link
Contributor

Choose a reason for hiding this comment

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

you can just pass the kwargs here (having a separate dict doesn't seem to buy much)

Copy link
Contributor

Choose a reason for hiding this comment

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

and you need to force the format here. again, bytes_io could easily contain a postscript image here, for example.

self.print_figure(bytes_io, **kw)
raw_bytes = bytes_io.getvalue()

mimetype_dict = {'png': 'image/png', 'svg': 'image/svg+xml',
Copy link
Contributor

Choose a reason for hiding this comment

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

see#14218 for how to get the mapping from the stdlib

@tdpetrou
Copy link
ContributorAuthor

tdpetrou commentedMar 17, 2020
edited
Loading

Made more changes toFigureCanvasBase._repr_html_ to account for retina display.

Currently, when retina is chosen as the figure format with%config InlineBackend.figure_format = 'retina' a png will always be output. This functionality is changed in this pull request to use theget_default_filetype of the canvas.

Thebbox_inches are set to 'tight' whenever%matplotlib inline has not been run. They are set to theInlineBackend.print_figure_kwargs['bbox_inches'] when inline activated.

All backends usercParams['savefig.format'] whenevercanvas.get_default_filetype is not implemented.

Backendnbagg is still interactive (if%matplotlib notebook was run) as it appears that ipython's display formatter gets triggered first which starts the application (I think?).

@anntzer
Copy link
Contributor

Ah, I missed the fact that ipython had an additional "retina" format. Ugh.

@@ -1595,6 +1595,73 @@ def __init__(self, figure):
self.toolbar = None # NavigationToolbar2 will set me
self._is_idle_drawing = False

def _repr_html_(self):
if not self.figure.axes and not self.figure.lines:
return

Choose a reason for hiding this comment

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

What if the figure only contains a Rectangle an arrow and a Text?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

True, I copied that from elsewhere. Is there a clean way to determine if the figure has changed at all?

Choose a reason for hiding this comment

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

Checkhttps://matplotlib.org/faq/howto_faq.html#check-whether-a-figure-is-empty
However, maybe even completely empty figures should return some html? After all, you get what you ask for, right?

Copy link
Member

Choose a reason for hiding this comment

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

What do you mean "changed"?

I agree with@ImportanceOfBeingErnest if we are going to have a repr, it should always showsomething even if that something is an empty image.

is_retina = False

import IPython
ip = IPython.get_ipython()

Choose a reason for hiding this comment

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

This would fail whenever IPython is not available.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Wouldn't_repr_html_ only get triggered for ipython?

Choose a reason for hiding this comment

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

I guess IPython invented this; but in principle anything can try to call such method if it's available.

Copy link
Member

Choose a reason for hiding this comment

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

The other way you can check this isif 'IPython' in sys.modules. That way if the user has not previously imported IPython we won't do it for them.

@ImportanceOfBeingErnest
Copy link
Member

This seems to pretty much reimplement the Ipykernel inline backend into matplotlib. I'm not convinced that this is the right approach, because it uses the IPython configuration system to determine the right parameters and thereby introduces a dependency on that config system which I don't think should be there.

What about adding aFigure.to_html(**savefigkw) method instead?

anntzer reacted with thumbs up emoji

@tdpetrou
Copy link
ContributorAuthor

@ImportanceOfBeingErnest The point of this is to work with jupyter notebooks whenever a figure is the last line of a cell without the need for%matplotlib inline.

The only reason IPython is accessed is to check whether the display is retina and whatbbox_inches is set to.

If you had ato_html method then you'd have to hook it into ipython somehow, which would use similar code.

@ImportanceOfBeingErnest
Copy link
Member

I think my problem is really about the two-way dependency introduced here. If IPython/IPykernel decides to introduce a new figure format"SchweineGulasch", is matplotlib expected to take it over as well? If they decide to change the defaultbbox_inches parameter, should matplotlib adapt to it?

But if we don't, then people will complain that the figures they produce with the IPykernel inline backend look different from the ones they create via matplotlib's_repr_html_.

@tdpetrou
Copy link
ContributorAuthor

Good point. It would be nice if ipython was made completely independent from matplotlib and dropped all of its code that had to do with it. matplotlib could then handle everything with the importing of pyplot and_repr_html_. Would be a much cleaner approach.

dpi = dpi * 2
fmt = self.get_default_filetype()
else:
bbox_inches = 'tight' # how to let user choose self.figure.bbox_inches?
Copy link
Member

Choose a reason for hiding this comment

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

This just should not be passed or set to None. That IPython inline defaults to'tight' has caused a tremendous amount of confusion over the years.

@tacaswell
Copy link
Member

I again agree with@ImportanceOfBeingErnest , the default implementation on the canvas base-class should not special case the inline backend and IPyhon's configuration system (just embedding a png at the native size would be fine). The support for that could be done in a companion PR into IPython to move their logic to the correct place. It seems fair for people who want the fancier controls from IPython have to use the IPython sourced backend.

There will also need to be a companion PR into ipympl so that we correctly defer to their canvas as well.

I am very 👍 on this PR in principle, but the technical interaction between Matplotlib and IPython/jupyter has always been complicated and we need to make sure that we are making it simpler for the devs while making it more streamlined for the users.

@tacaswelltacaswell added this to thev3.3.0 milestoneMar 18, 2020
@tdpetrou
Copy link
ContributorAuthor

Thanks@tacaswell for the feedback. I've changed approaches again and now defer all html output to ipython ifInlineBackend is in theconfigurables list. This greatly simplifies things. (Is there a better way to check for this?)

Otherwise, the default canvas format gets returned as HTML.

Essentially, this method will only be useful for those who don't run%matplotlib and don't import pyplot. As soon as either of those happen, ipython is in full control.

ib_list = [c for c in ip.configurables
if 'InlineBackend' in type(c).__name__]
if ib_list:
return
Copy link
Contributor

Choose a reason for hiding this comment

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

Just dived into IPython itself. It looks like we are also storing the sate on a function attribute (!!!).

In [1]: from IPython.core.pylabtools import configure_inline_supportIn [2]: configure_inline_support.current_backend---------------------------------------------------------------------------AttributeError                            Traceback (most recent call last)<ipython-input-2-eec19e858e54> in <module>----> 1 configure_inline_support.current_backendAttributeError: 'function' object has no attribute 'current_backend'In [3]: %matplotlib qtIn [4]: configure_inline_support.current_backendOut[4]: 'other'In [5]: %matplotlib inlineIn [6]: configure_inline_support.current_backendOut[6]: 'inline'

Not sure it is better though.

@Carreau
Copy link
Contributor

Definitively the separation IPython/Matplotlib is not great, and I'm happy to try to get better behavior on the IPython codebase.

The concern I have with__repr_html__ is that you may end up with duplicated figures if the last line of a cell return a figure. We went to relatively great length to make sure figurer get displayed when they are not the last in the cell; so if you end up having a call at end-of-cell, that happen to also return a fig/ax, then you will risk double display.

I don't like having to special case matplotlib either, and admittedly there is no really good way to know wether we are in inline mode or not. At some points, we likely want to sit-down for a couple of hours list the current behavior and plan for a progressive cleanup over the next few years.

@tdpetrou
Copy link
ContributorAuthor

Definitively the separation IPython/Matplotlib is not great, and I'm happy to try to get better behavior on the IPython codebase.

Heh, I'm about to submit a post on the matplotlib discourse stating how great this would be :)

Regarding the double display: Currently, I experience the double display whenever I have aplt.command in the cell along withfig at the bottom of the same cell. This new_repr_html_ would trigger double display if pyplot was imported or%matplotlib run and the last line was aplt.command that returned a figure (or another library that returned a figure using pyplot). I think this would be very rare.

Agreed, the current behavior is pretty convoluted and I think can be greatly simplified.

@jklymak
Copy link
Member

Closing in lieu of#17891, but feel free to reopen if I've misunderstood...

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@tacaswelltacaswelltacaswell left review comments

@CarreauCarreauCarreau left review comments

@anntzeranntzeranntzer left review comments

@ImportanceOfBeingErnestImportanceOfBeingErnestImportanceOfBeingErnest left review comments

Assignees
No one assigned
Projects
None yet
Milestone
v3.5.0
Development

Successfully merging this pull request may close these issues.

Add _repr_html_ for inline backend to eliminate need for %matplotlib inline
8 participants
@tdpetrou@story645@anntzer@ImportanceOfBeingErnest@tacaswell@Carreau@jklymak@QuLogic

[8]ページ先頭

©2009-2025 Movatter.jp