-
-
Notifications
You must be signed in to change notification settings - Fork 8.4k
Faster legend with location 'best' #8126
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
fefce87
94c4d9f
8c05491
d9e9f29
fe87068
a6c8d57
cd6877d
7ac1844
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| Path.intersects_bbox always treats the bounding box as filled | ||
| ````````````````````````````````````````````````````````````` | ||
|
|
||
| Previously, when ``Path.intersects_bbox`` was called with ``filled`` set to | ||
| ``False``, it would treat both the path and the bounding box as unfilled. This | ||
| behavior was not well documented and it is usually not the desired behavior, | ||
| since bounding boxes are used to represent more complex shapes located inside | ||
| the bounding box. This behavior has now been changed: when ``filled`` is | ||
| ``False``, the path will be treated as unfilled, but the bounding box is still | ||
| treated as filled. The old behavior was arguably an implementation bug. | ||
|
|
||
| When ``Path.intersects_bbox`` is called with ``filled`` set to ``True`` | ||
| (the default value), there is no change in behavior. For those rare cases where | ||
| ``Path.intersects_bbox`` was called with ``filled`` set to ``False`` and where | ||
| the old behavior is actually desired, the suggested workaround is to call | ||
| ``Path.intersects_path`` with a rectangle as the path: | ||
|
|
||
| from matplotlib.path import Path | ||
| from matplotlib.transforms import Bbox, BboxTransformTo | ||
| rect = Path.unit_rectangle().transformed(BboxTransformTo(bbox)) | ||
| result = path.intersects_path(rect, filled=False) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -558,14 +558,13 @@ def intersects_bbox(self, bbox, filled=True): | |
| :class:`~matplotlib.transforms.Bbox`. | ||
|
|
||
| *filled*, when True, treats the path as if it was filled. | ||
| That is, if one path completely encloses the other, | ||
| :meth:`intersects_path` will return True. | ||
| That is, if the path completely encloses the bounding box, | ||
| :meth:`intersects_bbox` will return True. | ||
|
|
||
| The bounding box is always considered filled. | ||
| """ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did this used to deal with non-affine transformations?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, I checked that. This is just a somewhat confusing way of getting a rectangle that represents the bounding box. |
||
| from .transforms import BboxTransformTo | ||
| rectangle = self.unit_rectangle().transformed( | ||
| BboxTransformTo(bbox)) | ||
| result = self.intersects_path(rectangle, filled) | ||
| return result | ||
| return _path.path_intersects_rectangle(self, | ||
| bbox.x0, bbox.y0, bbox.x1, bbox.y1, filled) | ||
|
|
||
| def interpolated(self, steps): | ||
| """ | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a change in behavior or clarification of current behavior?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a change in behaviour when filled=False, however this mode was only used in
legend.py, and in that context the old behaviour was incorrect: the bounding box (legend) is always filled even when the plot lines are not filled.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way to not change this behavior? This is a public method on a public so we must be careful about changing API. You will be surprised what behavior users have found and rely on 😈 .
On the other hand, the docstring was clearly a copy-paste from the method above and not quite right and I think it is reasonable to interpret 'intersects bbox' as 'is in or crosses boundary of bbox'. It that case this needs to be documented as an API change in https://github.com/matplotlib/matplotlib/tree/master/doc/api/api_changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name 'bounding box' already implies that the box represents the boundary of some more complex object which is somewhere inside the bounding box, so I think the new behavior makes more sense.
In order to retain the old behavior, it would be necessary to add an additional option, because we would actually need three use cases:
So it looks like that would also result in an API change, unless the
filledargument accepts more than two possible values.If you agree, I will keep the behavior as I've implemented it now and update
api_changes.