Skip to content

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

Closed
tacaswell wants to merge 1 commit into
matplotlib:masterfrom
tacaswell:api_remove_unused_attritue
Closed

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

Conversation

@tacaswell

@tacaswell tacaswell commented Nov 11, 2016

Copy link
Copy Markdown
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 API: changes Changes to the public API, typically requiring deprecation. status: needs revision labels Nov 11, 2016
@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:

@Kojoley Kojoley Nov 13, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

elif -> if

@NelleV

NelleV commented Dec 19, 2016

Copy link
Copy Markdown
Member

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

efiring commented Dec 19, 2016

Copy link
Copy Markdown
Member

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

timhoffm commented Apr 6, 2018

Copy link
Copy Markdown
Member

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 Changes to the public API, typically requiring deprecation. 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