-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
DOC: Correct subplot() doc #7760
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
Conversation
Creating a new subplot with a position which is entirely inside a | ||
pre-existing axes will trigger the larger axes to be deleted:: | ||
Creating a new subplot with axes that overlap pre-existing axes will | ||
trigger the pre-existing axes to be deleted:: |
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.
Something like "Creating a subplot will delete any pre-existing subplot that overlaps with it." seems clearer (possibly add "(if any)" at the end).
Good suggestion @anntzer |
Current coverage is 62.11% (diff: 100%)@@ master #7760 diff @@
==========================================
Files 174 174
Lines 56028 56028
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
- Hits 34805 34803 -2
- Misses 21223 21225 +2
Partials 0 0
|
lgtm |
@@ -976,8 +976,8 @@ def subplot(*args, **kwargs): | |||
|
|||
.. note:: | |||
|
|||
Creating a new subplot with a position which is entirely inside a | |||
pre-existing axes will trigger the larger axes to be deleted:: | |||
Creating a subplot will delete any pre-existing subplot that overlaps |
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.
Overlaps or fully overlaps?
Thanks @petehuang |
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.
Please hold off. I think that both the code and the documentation are incomprehensible.
I challenge anyone reading the proposed modification of the docstring, without poring over the code, to explain what "fully overlaps" means. Before reading the code, I was imagining that it might mean "encloses", as the original form of the docstring implied. Based on the Bbox method code, however, it means that the interiors intersect--that is, the intersection of the two rectangles encloses a finite area. Hence the subplot code is deleting any pre-existing subplots that have a finite intersection with the new subplot. Coinciding edges are not enough, because then the intersection encloses no area. |
Ok let me paraphrase to make sure I understand correctly: you're saying that it's not determined by axes lying over one another, but it's that the rectangles that the axes create intersect in some way? I just want to make sure we get this explanation and the docstring in plain english |
The practical problem is coming from the adverb "fully". The "overlaps" part is fine. The subtle distinction is that a shared boundary does not count as an "overlap". You could change "that fully overlaps with it" to "that has a finite area of overlap with it". Or "that overlaps with it beyond sharing a boundary". |
I knew what the function did (and I find this behavior extremely strange…). |
Thank you! |
Backported to |
Fixes #7393