Add improved modification of QuiverKey properties #18794
Conversation
| self._initialized = False | ||
| self.zorder = Q.zorder + 0.1 | ||
|
|
||
| def get_x(self): |
There was a problem hiding this comment.
Perhaps we can do without {get,set}_{x,y} and just stick to get_position (as for Text)? the backcompat X and Y would be kept for backcompat but can use straight lambdas (X = property(lambda self: self._text.get_position()[0], self.text.set_x) -- setting stale shouldn't really matter here as Text will propagate staleness to the parent Axes anyways)
There was a problem hiding this comment.
Agree. We only have get_x on Rectangle and FancyBboxPatch. Let's keep the API size down and only provide get/set_position.
| self._update_text_transform() | ||
| self.stale = True | ||
|
|
||
| @property |
There was a problem hiding this comment.
consider deprecating this, given the potential source of confusion?
|
|
||
| - ``QuiverKey.text`` | ||
| - ``QuiverKey.vector`` | ||
| - ``QuiverKey.verts`` |
There was a problem hiding this comment.
Perhaps someone may want e.g. to set usetex or math_fontfamily or whatnot on the Text; do you really want to deprecate access to it?
There was a problem hiding this comment.
Keeping text public is the pragmatic thing to do.
|
Since this Pull Request has not been updated in 60 days, it has been marked "inactive." This does not mean that it will be closed, though it may be moved to a "Draft" state. This helps maintainers prioritize their reviewing efforts. You can pick the PR back up anytime - please ping us if you need a review or guidance to move the PR forward! If you do not plan on continuing the work, please let us know so that we can either find someone to take the PR over, or close it. |
Additionally, add getters/setters for these properties, and use an `offset_copy` Transform to implement `labelsep`. This makes one less thing depending on the DPI change signal.
|
I finally got around to rebasing this, though I don't really remember why I started this PR in the first place. @timhoffm what do you think about the API changes here, mainly the comments from @anntzer? I also had to use |
|
Commented on @anntzer‘s suggestions. label: Unfortunately, Artist.set_label is tied to the specific meaning: The label an artist gets in a legend. I‘m against repurposing that function for another usecase. See also #27971 (comment). Can‘t we leave the |
Yes, that does seem possible; should we deprecate the |
|
Let's deprecate all the |
PR Summary
QuiverKeypreviously had several attributes that were saved from the constructor, but these were never linked to the internal Artists that were actually drawn.This adds getters and setters for said properties, and instead of a separate copy of the information, just grabs/sets it on the internal Text child. Additionally, it deprecates access to these internal children; other than
.text, they were always overwritten indraw, so there was no way to make real use of them.PR Checklist
pytestpasses).flake8on changed files to check).flake8-docstringsand runflake8 --docstring-convention=all).doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).