Skip to content

DOC: Correct subplot() doc - #7760

Merged
efiring merged 3 commits into
matplotlib:masterfrom
petehuang:subplot-docs
Jan 10, 2017
Merged

efiring merged 3 commits into
matplotlib:masterfrom
petehuang:subplot-docs

Conversation

@petehuang

Copy link
Copy Markdown
Contributor

Fixes #7393

Comment thread lib/matplotlib/pyplot.py Outdated
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::

Copy link
Copy Markdown
Contributor

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

@petehuang

Copy link
Copy Markdown
Contributor Author

Good suggestion @anntzer

@codecov-io

codecov-io commented Jan 7, 2017 •

Copy link
Copy Markdown

Current coverage is 62.11% (diff: 100%)

Merging #7760 into master will decrease coverage by <.01%

@@             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          

Powered by Codecov. Last update 1fa4dd7...4abaa24

@anntzer anntzer changed the title DOC: Correct subplot() doc [MRG+1] DOC: Correct subplot() doc Jan 7, 2017
@anntzer

anntzer commented Jan 7, 2017

Copy link
Copy Markdown
Contributor

lgtm

@tacaswell tacaswell added this to the 2.0.1 (next bug fix release) milestone Jan 7, 2017
Comment thread lib/matplotlib/pyplot.py Outdated
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

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.

Overlaps or fully overlaps?

@NelleV

NelleV commented Jan 8, 2017

Copy link
Copy Markdown
Member

Thanks @petehuang

@NelleV NelleV changed the title [MRG+1] DOC: Correct subplot() doc [MRG+2] DOC: Correct subplot() doc Jan 8, 2017

@efiring efiring left a comment

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.

Please hold off. I think that both the code and the documentation are incomprehensible.

@efiring

efiring commented Jan 8, 2017

Copy link
Copy Markdown
Member

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.

@petehuang

Copy link
Copy Markdown
Contributor Author

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

@efiring

efiring commented Jan 8, 2017

Copy link
Copy Markdown
Member

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

@NelleV

NelleV commented Jan 9, 2017

Copy link
Copy Markdown
Member

I knew what the function did (and I find this behavior extremely strange…).
I would actually raise a warning to the user when that happens, as this behavior is quite unexpected.

@efiring efiring changed the title [MRG+2] DOC: Correct subplot() doc DOC: Correct subplot() doc Jan 10, 2017
@efiring
efiring merged commit d75972e into matplotlib:master Jan 10, 2017
@efiring

efiring commented Jan 10, 2017

Copy link
Copy Markdown
Member

Thank you!

@QuLogic

QuLogic commented Jan 30, 2017 •

Copy link
Copy Markdown
Member

Backported to v2.0.x as 98c374d.

QuLogic pushed a commit that referenced this pull request Jan 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants