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

Move Text init to end of Annotation init.#15747

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 2 commits intomatplotlib:masterfromanntzer:annotationinit
Mar 24, 2020

Conversation

anntzer
Copy link
Contributor

PR Summary

... in case some kwargs (each of which is handled by calling a set_foo)
need to refer to arrow_patch).Closes#15745.

I nearly told the OP of#15745 that he needed to create an annotation without setting axes and figure because add_text (like add_line) would take care of it for him... but _add_text is private, for some reason (probably should be public, like add_line and friends?)

Small neighboring cleanup in a separate commit.

PR Checklist

  • Has Pytest style unit tests
  • Code isFlake 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

@timhoffm
Copy link
Member

timhoffm commentedNov 22, 2019
edited
Loading

Not sure if this is a good design pattern overall. We requireText to pass through arbitrary kwargs.Text() in turn splits__init__ into

def __init__(self, ..., kwargs):    Artist.__init__(self)    # set up attributes    self.update(kwargs)

Would that pattern be suitable forAnnotation itself as well?

@anntzer
Copy link
ContributorAuthor

That looks better to me, adapted accordingly.

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.

Isuppose this is ok.

A bit hesitant because this changes the code path for Text properties. They are not initialized viaText.__init__ anymore but via a defferedupdate(). In a way, this assumes something about the inner workings ofText. OTOH,t = Text(); t.update(kwargs) is public and the effect is (and I assume is guaranteed) to be equivalent toText(**kwargs). The former might just be (I think currently is not) a little more inefficient.

IMHO the really clean approach would be to passText properties toText.__init__() andupdate properties specific toAnnotation. But maybe we don't hae to overdo it here.

... in case some kwargs (each of which is handled by calling a set_foo)need to refer to arrow_patch.
@anntzer
Copy link
ContributorAuthor

Actually, instead of splitting Text init in two, I can move the whole init after setting up the arrow_patch, which should alleviate your concerns.

@QuLogicQuLogic added this to thev3.3.0 milestoneMar 24, 2020
@QuLogicQuLogic merged commit2ff579d intomatplotlib:masterMar 24, 2020
@anntzeranntzer deleted the annotationinit branchMarch 24, 2020 08:41
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@QuLogicQuLogicQuLogic approved these changes

@timhoffmtimhoffmtimhoffm approved these changes

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

Successfully merging this pull request may close these issues.

Failed to add annotate to figure
3 participants
@anntzer@timhoffm@QuLogic

[8]ページ先頭

©2009-2025 Movatter.jp