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

Cleanup matplotlib.use#13117

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:masterfromtimhoffm:use-cleanup
Feb 2, 2019
Merged

Conversation

timhoffm
Copy link
Member

PR Summary

@@ -1297,15 +1297,15 @@ def __exit__(self, exc_type, exc_value, exc_tb):
self.__fallback()


def use(arg, warn=False, force=True):
def use(backend, warn=False, force=True):
Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Technically, this is an API change. I'm 0.5 on this because it makes clear what the function does and what the argument is about. I doubt that someone has been doingmatplotlib.use(arg='agg') so I dare go without a deprecation and backward compatibility code. (But that position is open for discussion).

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this needs deprecation code, but an api change note perhaps?

Copy link
Member

Choose a reason for hiding this comment

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

I doubt that someone has been doingmatplotlib.use(arg='agg')

Users are very surprising ;) .

I'm -0.5 on this. It is definitely clearer, but is the improved clarity worth breaking even a handful of users and ruining their day?

Copy link
Contributor

@anntzeranntzerJan 7, 2019
edited by tacaswell
Loading

Choose a reason for hiding this comment

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

I actually think the answer is yes, it is; but if you insist

def_rename_parameter(since,old,new,func=None):"""    Decorator indicating that a function parameter is being renamed.    The actual implementation of *func* should use *new*, not *old*.    Examples    --------    ::        @_rename_parameter("3.1", "bad_name", "good_name")        def func(good_name): ...    """iffuncisNone:returnfunctools.partial(_rename_parameter,since,old,new)new_sig=inspect.signature(func)old_sig=new_sig.replace(parameters=[param.replace(name=old)ifparam.name==newelseparamforparaminnew_sig.parameters.values()])def_bind_or_none(signature,*args,**kwargs):try:returnsignature.bind(*args,**kwargs)exceptTypeError:returnNone@functools.wraps(func)defwrapper(*args,**kwargs):try:returnfunc(*args,**kwargs)exceptTypeError:new_ba=_bind_or_none(new_sig,*args,**kwargs)ifnew_baisnotNone:# Args match the new signature, so the TypeError was not raised# during argument binding; reraise it.raiseold_ba=_bind_or_none(old_sig,*args,**kwargs)ifold_baisNone:# Args match neither the old nor the new signature; reraise the# argument binding failure.raise# Move out of the except: block to avoid exception chaining.  The# function was called with the old signature.  Emit a deprecation# warning and call with the new signature.warn_deprecated(since,message=f"The{old!r} argument of "f"{func.__name__}() has been renamed{new!r} since "f"{since}; support for the old name will be dropped "f"%(removal)s.")returnfunc(*old_ba.args,**{keyifkeyisnotoldelsenew:valueforkey,valueinold_ba.kwargs.items()})returnwrapper

courtesy of the house.

Use:

@_rename_parameter("3.1", "arg", "backend")def use(backend, ...): ...

(The implementation is kind of overly general but also prepares further cases I want to implement such as deleting an argument or making arguments keyword-only...)

Copy link
Member

Choose a reason for hiding this comment

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

👍 to adding this to cbook as per phone conversation.

@timhoffmtimhoffm added this to thev3.1 milestoneJan 6, 2019
The first parameter of `matplotlib.use` has been renamed from *arg* to
*backend*. This will only affect cases where that parameter has been set
as a keyword argument. The common usage pattern as a positional argument
(``matplotlib.use('Qt5Agg')`` is not affected.
Copy link
Contributor

Choose a reason for hiding this comment

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

parenthesis

'standard'backend names:
backend : str
The backend to switch to. This can either be one of the standard
backend names:
Copy link
Contributor

Choose a reason for hiding this comment

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

standard backend names, which are case-insensitive: ... and remove the note below

@jklymak
Copy link
Member

jklymak commentedJan 15, 2019
edited
Loading

OK, is this waiting for the rename decorator? Feel free to change the labels back if I'm incorrect.

@anntzer
Copy link
Contributor

would prefer so, but it's not the end of the world if this one goes in first and I rebase the other (basically depends on how strongly@tacaswell feels about the temporary API break)

@timhoffm
Copy link
MemberAuthor

Can be reviewed, now that#13128 is in.

@@ -1184,13 +1184,13 @@ def __exit__(self, exc_type, exc_value, exc_tb):
@cbook._rename_parameter("3.1", "arg", "backend")
def use(backend, warn=False, force=True):
"""
Set the matplotlib backendto one of the known backends.
Select the matplotlib backendfor rendering.
Copy link
Contributor

Choose a reason for hiding this comment

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

not just for rendering -- also for GUI toolkit integration (in fact, mostly for that purpose, as I'd guess most uses are selecting qt5agg vs gtk3agg vs tkagg rather than agg vs cairo vs pdf...)

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

The interpretation of "rendering" should be broad here, as in "Render into a Qt window". The original docstring was more or less tautological. OTOH just "Set the matplotlib backend." is a bit short and does not help if you have no clue what a backend is for.

Better descriptions are welcome.

Copy link
Contributor

Choose a reason for hiding this comment

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

"Select the backend used for rendering and GUI integration."?

timhoffm reacted with thumbs up emoji
Copy link
Member

@NelleVNelleV left a comment

Choose a reason for hiding this comment

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

So sorry it took so long and thanks!

@anntzeranntzer merged commit615046f intomatplotlib:masterFeb 2, 2019
@timhoffmtimhoffm deleted the use-cleanup branchFebruary 2, 2019 14:53
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@tacaswelltacaswelltacaswell left review comments

@NelleVNelleVNelleV approved these changes

@anntzeranntzeranntzer approved these changes

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

Successfully merging this pull request may close these issues.

5 participants
@timhoffm@jklymak@anntzer@NelleV@tacaswell

[8]ページ先頭

©2009-2025 Movatter.jp