Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Add BackendRegistry singleton class#27719
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Uh oh!
There was an error while loading.Please reload this page.
lib/matplotlib/rcsetup.py Outdated
'pdf', 'pgf', 'ps', 'svg', 'template'] | ||
all_backends = interactive_bk + non_interactive_bk | ||
# Deprecation of module-level attributes using PEP 562 | ||
_deprecated_interactive_bk = backendRegistry.list_builtin(BackendFilter.INTERACTIVE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
If we didn't deprecate these 3 attributes, this block of changes would be replaced by
interactive_bk=backendRegistry.list_builtin(BackendFilter.INTERACTIVE)non_interactive_bk=backendRegistry.list_builtin(BackendFilter.NON_INTERACTIVE)all_backends=backendRegistry.list_builtin()
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Looks good to me.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I'll block based on#27719 (comment) though only as a note that INTERACTIVE_NON_WEB should be removed; anyone can dismiss once fixed.
c9ca79f
toea74018
CompareI've removed all of |
# Singleton | ||
backend_registry = BackendRegistry() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Now we havematplotlib.backends.registry.backend_registry
, which seems a bit redundant. Would it make sense to move this to_registry.py
(to keep files small), and then import intomatplotlib/backends/__init__.py
so we havematplotlib.backends.registry
as the instance?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I'd rather not. There is value in matching the the singleton instance namebackend_registry
to the class nameBackendRegistry
. Also the internal usage is frommatplotlib.backends.registry import backend_registry
, so the name is free standing. Onlyregistry
would be quite ambiguous (one could change to fully qualified usages only, but 🤷 ). And finally, the backend registry is used really rarely so that verbosity is not much of an issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I lean towards the explicit and verbose approach, but maybe there is some middle ground. If I keep the files named as they are but import intomatplotlib.backends.__init__
usingfrom .registry import BackendFilter, backend_registry
then the standard usage becomes
frommatplotlib.backendsimportbackend_registry
rather than
frommatplotlib.backends.registryimportbackend_registry
removing some verbosity.
This has the added advantage that you cannot import theBackendRegistry
class in the same way which is good as nobody should need the class, only the singleton instance.
I have implemented this in commit8f10a01.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
I've also added docstrings, whats new and API changes information. |
Were we also going to add a way for third-party backends to registering themselves? |
I very much think so. 😄 From the very top:
|
🐑 I failed my reading test for the day |
Yes, this PR is getting the ducks lined up ready for the real work in my next PR. |
Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>
8f10a01
toa5ae8df
CompareThe reduction is code coverage is due to changes I have had to make to |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
This is in preparation for#27663, moving the various hardcoded backend lists and dicts to a new
BackendRegistry
class that is intended to be the single source of truth for backends. There is no functional change.There is a singleton
backend_registry
instance of theBackendRegistry
class inregistry.py
. This is a similar implementation to that of the singletonrcParams
instance of theRcParams
class.BackendRegistry
includes the two public functions that are currently needed. There will eventually be alist_all
function to list all backends including the built-in ones, those obtained via the"module://blah.blah"
approach, and those registering themselves via entry points. The class internals will change too of course.There are still some hardcoded backend names in
_fix_ipython_backend2gui
, but this will be addressed as part of the wider#27663 implementation.I have not yet added an API change note. The
backend_registry
could be considered internal to Matplotlib except that IPython will also use it, so it could be thought of as fully public, or private with an exception for IPython. Other use of built-in backend lists by downstream libraries is probably restricted to use ofrcsetup.interactive_bk
,non_interactive_bk
andall_backends
. I have deprecated them in this PR to demonstrate that this is convoluted as they are module-level attributes that need to be deprecated using the recommendations of PEP 562. But probably I would be more inclined to keep them for the moment, until we are sure how this will all work in the end, and just pass through thebackend_registry
results.(Edited to change name of singleton instance)