NotificationsYou must be signed in to change notification settings
Fork7.9k
Star21.3k

[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.

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

Jump to bottom

Closed

tacaswell wants to merge1 commit intomatplotlib:masterfromtacaswell:api_remove_unused_attritue

Closed

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

tacaswell wants to merge1 commit intomatplotlib:masterfromtacaswell:api_remove_unused_attritue

Conversation

Copy link

Member

tacaswell commentedNov 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.

API: remove unused attribute on Annotation

7947dda

tacaswell added API: changes status: needs revision labels

Nov 11, 2016

tacaswell added this to the2.1 (next point release) milestone

Nov 11, 2016

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

Copy link

Member

NelleV commentedDec 19, 2016

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

NelleV changed the title~~API: remove unused attribute on Annotation~~[WIP] API: remove unused attribute on Annotation

Dec 19, 2016

Copy link

Member

This dates back more than a year, to#4178, so thearrow 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 thearrow attribute has beenNone 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.