Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
timhoffm commentedNov 22, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Not sure if this is a good design pattern overall. We require
Would that pattern be suitable for |
That looks better to me, adapted accordingly. |
There was a problem hiding this 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.
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. |
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