Skip to content

Remove matplotlib rc overrides. #14

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

Merged
merged 5 commits into from
May 20, 2022
Merged

Remove matplotlib rc overrides. #14

merged 5 commits into from
May 20, 2022

Conversation

fperez
Copy link
Member

@fperez fperez commented May 12, 2022

@ccordoba12
Copy link
Member

Just a quick comment to say that this is quite handy for us in Spyder, so I hope it's not removed in the future. I'm fine with leaving it empty though because we can fill it on our side.

@fperez
Copy link
Member Author

fperez commented May 13, 2022

Agreed @ccordoba12 - I didn't intend to remove b/c I figured someone out there would be likely to use it, but I've added clarifying language both in the help string and comments to this effect, so down the road nobody has the temptation to "clean up" and nuke it. Thx for the feedback!

@ccordoba12
Copy link
Member

ccordoba12 commented May 13, 2022

Great! Thanks for adding that extra comment @fperez!

To give a bit more of context, the thing is we make heavy use of the inline backend because we have a dedicated pane to display figures that come from it:

http://docs.spyder-ide.org/current/panes/plots.html

@fperez
Copy link
Member Author

fperez commented May 13, 2022

Makes sense, that's an excellent use case and it provides great UX. Thanks for the extra context!

@fperez
Copy link
Member Author

fperez commented May 18, 2022

Given this is now being reported by other mpl users, and @tacaswell's approval (thx!) I'm inclined to merge this.

Unless someone has a reason not to in the next day or two, I'm happy to do it. I haven't touched our actual code in ages so I don't want to step on anyone's toes, but I'll take silence as acquiescence :)

Once merged, it would be nice to cut a release in the next few weeks - I have a class coming up in the fall (as will many others) with over 1,000 students where this has caused headaches in the past, would be nice to not have to deal with it.

@juanitorduz
Copy link

It would be great to have a new release to be able to use matplotlib > 3.5.1 :) Thanks !

@fperez
Copy link
Member Author

fperez commented Aug 15, 2022

@juanitorduz can you clarify your comment? Is there a version-specific issue with matplotlib you refer to? We do want to make a release b/c of these other issues, but I'm using mpl 3.5.3 just fine, so I'd like to understand what you're referring to...

@tacaswell
Copy link
Contributor

I think @juanitorduz is asking for a release of matplotilb-inline with this PR in it.

@fperez
Copy link
Member Author

fperez commented Aug 15, 2022

Ah, ok, fair enough - that's indeed the plan :).

@juanitorduz
Copy link

I think @juanitorduz is asking for a release of matplotilb-inline with this PR in it.

Exactly 😅 ! Thank you for the info! 🚀

@fperez
Copy link
Member Author

fperez commented Aug 16, 2022

@juanitorduz - all set; in #16 we're finalizing the details, but I should have this up soon on PyPI. Thanks for the nudge :)

@mgeier
Copy link

mgeier commented Sep 3, 2022

Thanks @fperez for removing those RC overrides! I have suffered from them for years (see e.g. ipython/ipykernel#267 (comment)), even more so now that local ipython_kernel_config.py files are not supported anymore (ipython/ipython#13663).

With the latest matplotlib-inline it is finally possible to use matplotlibrc files, which is great!

The figure.dpi setting is still wrong by default (for notebooks), but at least now it is much closer to the correct value (100 vs. 96). And now it's easy to fix this with a matplotlibrc containing figure.dpi: 96.

@mgeier
Copy link

mgeier commented Sep 3, 2022

Could the wrong figure.dpi be fixed by default via a Matplotlib theme, see ipython/ipython#9710?

@fperez
Copy link
Member Author

fperez commented Sep 4, 2022

@mgeier - given we are not expliciltly setting the figure dpi anywhere anymore, and that I don't think we should embark on a custom theme, do you have any further thoughts on what we should do from our (as in iptyhon/mpl-inline) end on this?

If so, please go ahead and open a new issue on this so we can look into it. And thanks for your input on all these related issues!

@mgeier
Copy link

mgeier commented Sep 18, 2022

do you have any further thoughts on what we should do from our (as in iptyhon/mpl-inline) end on this?

I still think that it would be great if figure.dpi could somehow be 96 by default. But I don't know how this could be achieved. Maybe that's not feasible at all.

For now, I'm fine with manually creating a matplotlibrc file, which I've documented there:

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.

5 participants