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

Deprecate parameter props of Shadow#16098

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 1 commit intomatplotlib:masterfromtimhoffm:fix-tick
Mar 24, 2020

Conversation

timhoffm
Copy link
Member

@timhoffmtimhoffm commentedJan 4, 2020
edited
Loading

PR Summary

I was about to deprecate**kwargs with the previous PR on unused parameters. But according to the docs, kwargs should be supported.

Note: This has been in there since 2004, so I'm refraining from backporting due to lack of significance.

@timhoffmtimhoffm changed the titleFix passing kwargs to TicksFix passing kwargs to ShadowJan 4, 2020
@timhoffmtimhoffm added this to thev3.3.0 milestoneJan 4, 2020
@ImportanceOfBeingErnest
Copy link
Member

What would be the usecase for those kwargs? Since they will be overridden afterwards anyways (i.e.Shadow(...., color="red") willnot be red), it seems one should rather deprecate passing of kwargs.

@timhoffm
Copy link
MemberAuthor

Fair point. Actually, the properties for the shadow can be passed viaprop, see e.g.https://matplotlib.org/devdocs/gallery/text_labels_and_annotations/demo_text_path.html; and they are not overridden.

I've now deprecated**kwargs. Could be debatable if we should deprecateprop instead and make**kwargs work, but**kwargs didn't do anything, so they are the easier deprecation.

This would now fit into#16096 (hence the particular deprecation comment). But I'll leave this standalone and merge the conflict in case anybody has different opinions onprop vs. `**kwargs'.

@anntzer
Copy link
Contributor

I think deprecating props and making kwargs work looks better (it's more consistent with basically all mpl APIs...) but won't insist on it either.

@timhoffm
Copy link
MemberAuthor

Actually, there's a suprising number ofprop(s) parameters throughout the library. But I agree**kwargs would be the more common one.

@timhoffmtimhoffm changed the titleFix passing kwargs to ShadowDeprecate parameter props of ShadowJan 5, 2020
@timhoffm
Copy link
MemberAuthor

Gone for deprecatingprops. Shadow is the only class that uses it in the constuctor and as attribute.

@timhoffm
Copy link
MemberAuthor

@anntzer Do you want to re-review because the content of the PR changed completely after your approval?

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.

lgtm

@QuLogicQuLogic merged commit7b0cfe4 intomatplotlib:masterMar 24, 2020
@timhoffmtimhoffm deleted the fix-tick branchMarch 24, 2020 09:37
QuLogic added a commit to QuLogic/matplotlib that referenced this pull requestJun 17, 2020
Starting withmatplotlib#16098, extra keyword arguments are now processed, butthat means that default colours and alpha are not set. Instead, thoseproperties should only be overridden if specified.
@QuLogicQuLogic mentioned this pull requestJun 17, 2020
2 tasks
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@ImportanceOfBeingErnestImportanceOfBeingErnestImportanceOfBeingErnest left review comments

@QuLogicQuLogicQuLogic approved these changes

@anntzeranntzeranntzer approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
v3.3.0
Development

Successfully merging this pull request may close these issues.

4 participants
@timhoffm@ImportanceOfBeingErnest@anntzer@QuLogic

[8]ページ先頭

©2009-2025 Movatter.jp