Skip to content

[WIP] API: remove unused attribute on Annotation #7445

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

[WIP] API: remove unused attribute on Annotation #7445

wants to merge 1 commit into from
+0 −9

Conversation

tacaswell
Copy link
Member

Pretty sure this is fall out from @efiring cleaning up the arrow handling in annotation to always use the fancy arrows.

Definitely needs docs, might need deprecation cycle.

Posting this due to almost losing a whole pile of stashes today.

@tacaswell tacaswell added this to the 2.1 (next point release) milestone Nov 11, 2016
Kojoley
Kojoley reviewed Nov 13, 2016
View reviewed changes
lib/matplotlib/text.py
@@ -2366,8 +2359,6 @@ def get_window_extent(self, renderer=None):
text_bbox = Text.get_window_extent(self, renderer=renderer)
bboxes = [text_bbox]

if self.arrow is not None:
bboxes.append(arrow.get_window_extent(renderer=renderer))
elif self.arrow_patch is not None:
Copy link
Member

Choose a reason for hiding this comment

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

elif -> if

@NelleV
Copy link
Member

NelleV commented Dec 19, 2016

I think there is a need for a deprecation cycle here. We are removing a public attribute.

@NelleV NelleV changed the title API: remove unused attribute on Annotation [WIP] API: remove unused attribute on Annotation Dec 19, 2016
@efiring
Copy link
Member

efiring commented Dec 19, 2016

This dates back more than a year, to #4178, so the arrow attribute has been vestigial and non-functional throughout the 1.5 series. I don't see any point in putting in a deprecation cycle now--nor would I have supported it as part of #4178. We can tie ourselves in knots forever trying to guard against any possible breakage as matplotlib evolves. That would be unwise. A little judgment of the tradeoffs is needed.

Given that the arrow attribute has been None throughout the 1.5 cycle, I don't think its removal now even needs a documentation entry--why bother users with the information that something they never used, and that they couldn't have used in the last release even if they had wanted to, is gone?

The actual user-facing API change in #4178 is documented in the 1.5 API changes doc section. Presumably this was in a subsequent PR.

@tacaswell tacaswell modified the milestone: 2.1 (next point release) Aug 29, 2017
@anntzer anntzer mentioned this pull request Mar 17, 2018
Merged
6 tasks
@timhoffm
Copy link
Member

timhoffm commented Apr 6, 2018

Replaced by #10818.

@timhoffm timhoffm closed this Apr 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kojoley Kojoley Kojoley left review comments

Assignees
No one assigned
Labels
API: changes status: needs revision
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@tacaswell @NelleV @efiring @timhoffm @Kojoley