NotificationsYou must be signed in to change notification settings
Fork8.1k
Star22.1k

Add explicit registration of units in examples#23508

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

Jump to bottom

Closed

QuLogic wants to merge1 commit intomatplotlib:mainfromQuLogic:fix-docs

Closed

Add explicit registration of units in examples#23508

QuLogic wants to merge1 commit intomatplotlib:mainfromQuLogic:fix-docs

Conversation

Copy link

Member

QuLogic commentedJul 28, 2022

PR Summary

Starting insphinx-gallery/sphinx-gallery#890, sphinx-gallery started clearing Matplotlib's unit registry. But sincebasic_units was already imported, its register-on-import never happened for subsequent examples, breaking them.

PR Checklist

Tests and Styling

[n/a] Has pytest style unit tests (andpytest passes).
IsFlake 8 compliant (installflake8-docstrings and runflake8 --docstring-convention=all).

Documentation

New features are documented, with examples if plot related.
[n/a] New features have an entry indoc/users/next_whats_new/ (follow instructions in README.rst there).
[n/a] API changes documented indoc/api/next_api_changes/ (follow instructions in README.rst there).
Documentation is sphinx and numpydoc compliant (the docs shouldbuild without error).

Add explicit registration of units in examples

31618d3

Starting insphinx-gallery/sphinx-gallery#890,sphinx-gallery started clearing Matplotlib's unit registry. But since`basic_units` was already imported, its register-on-import neverhappened for subsequent examples, breaking them.

QuLogic added the Documentation label

Jul 28, 2022

QuLogic added this to thev3.5.3 milestone

Jul 28, 2022

QuLogic mentioned this pull request

Jul 28, 2022

Fix matplotlib resetter _reset_matplotlibsphinx-gallery/sphinx-gallery#890

Merged

story645 approved these changes

Jul 29, 2022

View reviewed changes

jklymak reviewed

Jul 29, 2022

View reviewed changes

examples/units/basic_units.py



		units.registry[BasicUnit]=units.registry[TaggedValue]=BasicUnitConverter()
		defregister_units():

Copy link

Member

jklymakJul 29, 2022

Choose a reason for hiding this comment

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

I'm still a little tan about who is incorrect here. It really seems sphinx gallery should spawn a new process for each example so the import happens again. However, I can see why that would be slow. It does make me extremely suspicious about changing behaviour via a registry that gets set at import time like this... (and yes we do it internally for dates and categorical as well).