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

Refactor backend loading#9551

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
anntzer merged 1 commit intomatplotlib:masterfromanntzer:backend-loading
May 29, 2018
Merged

Conversation

anntzer
Copy link
Contributor

@anntzeranntzer commentedOct 24, 2017
edited
Loading

This is preliminary work towards fixing the runtime detection of default
backend problem (the first step being to make it easier to check whether
a backendcan be loaded). No publically visible API is changed.

Backend loading now defines default versions ofbackend_version,
draw_if_interactive, andshow using the same inheritance strategy as
builtin backends do.

The_Backend class had to be moved to the end of the module as
FigureManagerBase needs to be defined first. TheShowBase class had
to be moved even after that as it depends on the_Backend class.

PR Summary

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@anntzeranntzerforce-pushed thebackend-loading branch 2 times, most recently from81d7c52 toc1dfd56CompareOctober 24, 2017 07:46
@anntzeranntzer self-assigned thisOct 24, 2017
@anntzeranntzer reopened thisOct 25, 2017
@anntzeranntzer removed their assignmentOct 25, 2017
@anntzeranntzer mentioned this pull requestNov 16, 2017
@@ -3362,3 +3250,129 @@ def set_message(self, s):
Message text
"""
pass

Copy link
Member

Choose a reason for hiding this comment

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

Is this block just a big copy/paste? (so we don't have to bother reviewing the code if it is)

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

It is,except for the fact that the FigureManager attribute now defaults to FigureManagerBase.

@anntzeranntzerforce-pushed thebackend-loading branch 2 times, most recently from33fb09d to56858aeCompareNovember 17, 2017 21:36
@anntzer
Copy link
ContributorAuthor

@jklymak If you want to help getting#9795 in, this is the first step towards it :) Just rebased it (on top of#11305, to avoid immediately yet another rebase).

backend_name = (name[9:] if name.startswith("module://")
else "matplotlib.backends.backend_{}".format(name.lower()))
backend_mod = importlib.import_module(backend_name)
Backend = type("Backend", (_Backend,), vars(backend_mod))
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a comment explaining this bit of magic? I get that it returns a base _Backend class, but am not quite sure whatvars(backend_mod) does.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

vars(obj) returns the attributes of obj as a dict. In the case of a module, that's basically the module's contents.

So effectively it's as if I'm running all of the module's code (of course, not a second time) from within aclass Backend(_Backend): ... context, and refetching attributes from that class. The point being that the base _Backend class can provide default implementations of backend_version, show, draw_if_interactive through inheritance rather than pylab_setup having to do it manually. Will add a comment to that effect, but does that sound good?

Copy link
Member

Choose a reason for hiding this comment

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

yeah, not objecting to the magic at all, but I think a short comment to help those of us with lesser python skills know whats going on would help make sure the code stays maintained...

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

added comment.

jklymak reacted with thumbs up emoji
@tacaswelltacaswell added this to thev3.0 milestoneMay 26, 2018
# Create a local Backend class whose body corresponds to the contents of
# the backend module. This allows the Backend class to fill in the missing
# methods through inheritance.
Backend = type("Backend", (_Backend,), vars(backend_mod))
Copy link
Member

Choose a reason for hiding this comment

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

So we create the class, use the inheritance to fill in the missing functions and then return class level functions (which 'forget' they were part of the class)?

That is quite clever.

To check, if a backend is already using@_Backend.export then there should be nothing extra we need to provide?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Indeed, backends that were using@_Backend.export don't really need that anymore (the contents of the local _Backend class could just be dumped at the module level (or alternatively they could just do without the temporary creation of a new Backend class))... except for one thing: the _Backend class still allows inheritance between backends, so that e.g. _BackendQt5Agg can inherit from _BackendQt5.

@@ -1699,8 +1699,7 @@ def pstoeps(tmpfile, bbox=None, rotated=False):
shutil.move(epsfile, tmpfile)


class FigureManagerPS(FigureManagerBase):
pass
FigureManagerPS = FigureManagerBase
Copy link
Member

Choose a reason for hiding this comment

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

There is a very subtle API change in here if people are looking at__name__,__module__, or__file__ of these classes. I am not sure if that is worth an API change note or not.

Copy link
ContributorAuthor

@anntzeranntzerMay 26, 2018
edited
Loading

Choose a reason for hiding this comment

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

I added an API note. The real intent here is the following:

I think the "correct" way to refer to a backend's FigureCanvas, FigureManager, etc. class is under the unsuffixed names (FigureCanvas instead of FigureCanvasPS, etc.) The reason is that 1) these unsuffixed names need to be defined anyways for the backend machinery to pick them up, and 2) some of these classes (FigureManager, Toolbar, though not FigureCanvas of course) can be shared between multiple backends (for example, the xxxcairo backends and mplcairo just reuses the FigureManager classes for each of the GUI toolkits).

So sure, I can define (e.g.) FigureManagerQt5 and FigureManagerQt5Agg and FigureManagerQt5Cairo and FigureManagerMplCairoQt to all be aliases (or trivial subclasses) of one another, but that's just muddying the picture IMO; promoting the use of the same FigureManager name everywhere seems clearer.

@anntzeranntzerforce-pushed thebackend-loading branch 2 times, most recently from7b8b41b to36fab4cCompareMay 26, 2018 00:41
@tacaswell
Copy link
Member

This needs a rebase.

@anntzer Please self-merge once CI passes.

@anntzer
Copy link
ContributorAuthor

Self-merging per#9551 (comment).

@anntzeranntzer merged commitad44a7e intomatplotlib:masterMay 29, 2018
@anntzeranntzer deleted the backend-loading branchMay 29, 2018 00:31
@anntzeranntzer mentioned this pull requestSep 7, 2019
6 tasks
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@dstansbydstansbydstansby left review comments

@tacaswelltacaswelltacaswell approved these changes

@jklymakjklymakjklymak approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v3.0.0
Development

Successfully merging this pull request may close these issues.

4 participants
@anntzer@tacaswell@jklymak@dstansby

[8]ページ先頭

©2009-2025 Movatter.jp