search

astoff / comint-mime

Display graphics and other MIME attachments in Emacs shells
67 stars 6 forks source link

bug: matplotlib rcParams modified if comint-mime-prefer-svg is non-nil #23

Closed pank closed 1 month ago

pank commented 3 months ago

It's great that svg support has been added!

But modifying matplotlib rcParams is a bug IMHO, as it changes the current mpl style (which might have set the facecolor) and therefore (potentially) changes the style of saved figures (plt.style is used for theming and does so my changing the rcParams).

Instead, the facecolor could be set for matplotlib_inline only, e.g. (in ipython_config.py syntax):

c.InlineBackend.print_figure_kwargs.update({"facecolor": (0, 0, 0, 0),})

However, if the axes facecolor is set this creates funny results where the plotting area has one facecolor but the surrounding area is transparent (e.g. axis labels and titles). I don't know if print_figure can change axes facecolors. IMHO, even changing the matplotlib_inline figure facecolor is a bit of a can of worms... (And if opinionated settings are applied more or less silently, why not get rid of the horrible tight layout default setting at the same time ('bbox_inches': None)?)

astoff commented 1 month ago

Sorry for the delay to respond, I hope I will have time to have a look in the coming week.

astoff commented 1 month ago

Okay, I see now the issues and I agree that your suggestion is an improvement, so I'll apply it if I don't find a better solution.

A better solution would be to make the SVG be "totally adaptive" (transparent background and lines / text color that change according to the user's theme) or else not adaptive at all. It's unclear to me that this is possible.

And if opinionated settings are applied more or less silently

I guess the goal here would be to just make it "work out of the box", so adding a workaroud for really serious problems seems justified.

why not get rid of the horrible tight layout default setting at the same time ('bbox_inches': None)

That one is an opinion indeed... So I'd rather not touch it.

astoff commented 1 month ago

Okay, I removed the Matplotlib configuration and replaced it with the following hint in comint-mime-prefer-svg docstring.

Using a theme with dark background can render some SVGs unreadable. If you experience this, try setting `comint-mime-image-props' to (:foreground \"black\" :background \"white\"). Alternatively, configure the SVG creator to produce images that play well with your theme.

pank commented 1 month ago

IMHO it is best to only change matplotlib_inline settings, not matplotlib rcParams.

If I load e.g. the 'dark_background' stylesheet and then enable SVG output in comint_mime, it could change my saved images. Using only matplotlib_inline settings, the user's matplotlib rcParams aren't touched (and if they are it's an upstream bug).