Skip to content

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

Closed
tacaswell wants to merge 1 commit intomatplotlib:masterfrom
tacaswell:api_remove_unused_attritue
Closed

[WIP] API: remove unused attribute on Annotation#7445
tacaswell wants to merge 1 commit intomatplotlib:masterfrom
tacaswell:api_remove_unused_attritue

Conversation

@tacaswell
Copy link
Copy Markdown
Member

@tacaswell tacaswell commented Nov 11, 2016

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
Comment thread lib/matplotlib/text.py

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

@Kojoley Kojoley Nov 13, 2016

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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