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

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

Merged
QuLogic merged 17 commits intomatplotlib:mainfromianthomas23:backend_registry_enum
Mar 9, 2024

Conversation

ianthomas23
Copy link
Member

@ianthomas23ianthomas23 commentedJan 30, 2024
edited
Loading

This is in preparation for#27663, moving the various hardcoded backend lists and dicts to a newBackendRegistry class that is intended to be the single source of truth for backends. There is no functional change.

There is a singletonbackend_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. Thebackend_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)

martinRenou reacted with hooray emojimartinRenou reacted with heart emoji
'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)
Copy link
MemberAuthor

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()

Copy link
Member

@timhoffmtimhoffm left a 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.

anntzer
anntzer previously requested changesFeb 8, 2024
Copy link
Contributor

@anntzeranntzer left a 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.

@ianthomas23
Copy link
MemberAuthor

I've removed all ofINTERACTIVE_NON_WEB and put the special case for webagg/nbagg back inpyplot.py as requested by@anntzer. Also rebased to pick up other fixes so hopefully all CI will pass.



# Singleton
backend_registry = BackendRegistry()
Copy link
Member

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?

Copy link
Member

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.

ianthomas23 reacted with thumbs up emoji
Copy link
MemberAuthor

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.

@ianthomas23
Copy link
MemberAuthor

I've also added docstrings, whats new and API changes information.

@tacaswelltacaswell added this to thev3.9.0 milestoneFeb 14, 2024
@tacaswell
Copy link
Member

Were we also going to add a way for third-party backends to registering themselves?

@timhoffm
Copy link
Member

I very much think so. 😄 From the very top:

This is in preparation for#27663, ([MNT]: Move Matplotlib backend mapping from IPython and support backends self-registering)

@tacaswell
Copy link
Member

I very much think so. 😄 From the very top:

🐑 I failed my reading test for the day

@ianthomas23
Copy link
MemberAuthor

I very much think so. 😄 From the very top:

This is in preparation for#27663, ([MNT]: Move Matplotlib backend mapping from IPython and support backends self-registering)

Yes, this PR is getting the ducks lined up ready for the real work in my next PR.

@ianthomas23
Copy link
MemberAuthor

The reduction is code coverage is due to changes I have had to make to_safe_pyplot_import. This function is not tested and is not used anywhere in the Matplotlib code base. It was added in#18316 (August 2020) and the one and only use of it was removed in#22925 (May 2022). So given that the function is private (leading underscore), not used and not tested, I think it should be removed.

timhoffm and tacaswell reacted with thumbs up emoji

@tacaswelltacaswell added the Release criticalFor bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. labelMar 6, 2024
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@timhoffmtimhoffmtimhoffm approved these changes

@tacaswelltacaswelltacaswell approved these changes

@QuLogicQuLogicQuLogic approved these changes

@anntzeranntzeranntzer left review comments

Assignees
No one assigned
Labels
Release criticalFor bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.topic: canvas and figure managertopic: pyplot APItopic: rcparams
Projects
None yet
Milestone
v3.9.0
Development

Successfully merging this pull request may close these issues.

5 participants
@ianthomas23@tacaswell@timhoffm@QuLogic@anntzer

[8]ページ先頭

©2009-2025 Movatter.jp