ping @stefanv @michaelpacer

There is still an indentation problem, but I am not seeing it...

When I look at the docstring, it looks like exactly what Stefan and I were talking about (and what I'd been looking for). But the rendered image included in the PR seems to cut off before the new example is displayed.

Fortunately, that image doesn't matter while the docstring does. Thanks @NelleV!

Can you update the figure at the top?

The AppVeyor fail appears unrelated.

I am not sure what figure you are referring to.

Of the rendered documentation in the first post (not in the docs themselves.)

Here it is (it's tiny as I had to zoom out a lot to be able to take a reasonable size screenshot)

subplots

Sorry to be late in commenting on this, but I am wondering whether you could condense the docstring by treating nrows and ncols together, and similarly with sharex and sharey. There is value in making docstrings as concise as possible without losing important info.

@efiring nrows and ncols, certaintly. The sharex sharey condensed is actually very hard to read (and to write)

Maybe something like the following:

sharex, sharey : bool or string, {'row', 'col', 'all', 'none'}, default is False
 Controls sharing of properties among x or y axes. Axis properties may
 be shared among all subplots (True or 'all'), among none (False or 'none'),
 or among subplots by 'row' or by 'column'. When subplots have shared y
 axes along a row, the y tick labels have their `visible` property set to False
 on all but the first column; similarly, when subplots have shared x axes
 along a column, only the x tick labels are visible only on the bottom 
 subplot.

I tried to just push a commit with my pedandtic word changes, but it would not let me.

index de97b5d..2205698 100644
--- a/lib/matplotlib/figure.py
+++ b/lib/matplotlib/figure.py
@@ -1056,7 +1056,7 @@ class Figure(Artist):
 squeeze : bool, default: True
 - If True, extra dimensions are squeezed out from the returned
- axis object:
+ object:
 - if only one subplot is constructed (nrows=ncols=1), the
 resulting single Axes object is returned as a scalar.
diff --git a/lib/matplotlib/pyplot.py b/lib/matplotlib/pyplot.py
index d304876..e112a27 100644
--- a/lib/matplotlib/pyplot.py
+++ b/lib/matplotlib/pyplot.py
@@ -1077,7 +1077,7 @@ def subplots(nrows=1, ncols=1, sharex=False, sharey=False, squeeze=True,
 subplots of the first column.
 squeeze : bool, optional, default: True
- - If True, extra dimensions are squeezed out from the returned Axes
+ - If True, extra dimensions are squeezed out from the returned
 object:
 - if only one subplot is constructed (nrows=ncols=1), the

@tacaswell that's weird: the tick box to allow edits from maintainers is ticked...

Before I do yet another commit, can you be more specific about the word change you'd like? The review and patch aren't the same.

@NelleV, and other reviewers, please reconsider your rejection of my attempt to make a much shorter docstring. I believe we need to think again about what makes a good docstring. Consider what you want to see, either in the terminal or in the browser: a concise description, with the most critical information appearing early. It should not insult the intelligence of the viewer, so it should minimize unnecessary repetition. Some standardization of structure, such as that imposed by numpydoc, is OK, but the text should look like it was written by a person, not an algorithm.

With those ideas in mind, I think that we should use lists sparingly, with minimal nesting, because they gobble space. And I don't see that a combined description of sharex and sharey is necessarily any harder or slower to read and digest than the present separate description. Surely my draft suggestion can be improved, but I think it illustrates a better path.

I am inclined to agree with @efiring about the fact that a too long or a too repetitive docstring can be hard to read or navigate, and that we should value docstring conciseness when it does not harm the docstring readibility nor the informations it provides. Nonetheless I still find first level lists to be useful to provide fast and random access to descriptive information.

In the present case, as the description of sharex and sharey parameters overlap significantly, I believe one could achieve some kind of factorization. Here is a draft that is inspired from both @NelleV and @efiring proposals:

 sharex, sharey : bool or {'none', 'all', 'row', 'col'}, default: False
 Controls sharing of properties among x (`sharex`) or y (`sharey`) axes:
 - If True or 'all', x- or y-axis will be shared among all subplots.
 - If False or 'none', all subplot x- or y-axis will be independant.
 - If 'row', each subplot row will share an x- or y-axis.
 - If 'col', each subplot column will share an x- or y-axis.
 When subplots have a shared x-axis along a column, only the x tick
 labels of the bottom subplot are visible. Similarly, when subplots
 have a shared y-axis along a row, only the y tick labels of the first
 column subplot are visible.

It is 3 lines longer than the super squeezed version, but only half the length of the non-factorized one. To be honest, I struggled a bit to fit each item of the list into a single (PEP8) line: I hope English is still correct.

@afvincent, thank you, that looks good to me. Recommended tweak: Each leading "If" could be removed, and the first comma replaced by a colon, e.g.,

 - True or 'all': x- or y-axis will be shared among all subplots.

In the second item (False), "all" -> "each".

I agree that shorter docstrings are in general better.
I've pushed the change.

Where I am more concerned is the amount of comments and debates on a pull request that original meant to add an example to the docstring. With half of the effort on that pull request, we probably could have done 5 more improvements on other docstrings.

On 2016年10月12日 6:55 AM, Nelle Varoquaux wrote:

Where I am more concerned is the amount of comments and debates on a
pull request that original meant to add an example to the docstring.
With half of the effort on that pull request, we probably could have
done 5 more improvements on other docstrings.

Good point. This is a real problem. I think that what I should have
done was to wait for your PR to be merged and then use a separate PR to
propose additional modifications.

Where I am more concerned is the amount of comments and debates on a pull request that original meant to add an example to the docstring. With half of the effort on that pull request, we probably could have done 5 more improvements on other docstrings.

That's not all it did though, and sometimes things need to be discussed before they get to a good consensus. It's easier to do so on one PR than a whole bunch of them, especially if they're all working on the same code/docs.

@QuLogic First, these discussions should not happen on PRs, as it doesn't include the whole community. Second, that's not the first PR were this happens: most of the documentation PRs I have seen take a very long time to be merged. It is much easier to nitpick and bikeshed on a docstring than on tiny code patch that fixes a bug hard to track (#7214 has been opened for 9 days, yet only has one approval and potentially three people looking at it).
And that does account for the "discouragement factor" of the potentially new contributor, who'll probably never submit a patch again after seeing this.

You should check Pieter Hintjens' view on the question: http://hintjens.com/blog:106
Thought I don't agree with him that all pull requests should merge without doing code review, his opinion is worth reading and thinking about.

I think that the threshold on documentation only PRs should be "Does this make it better and is not wrong?", if yes then merge.

Documentation seems to be much harder to collaborate on via PR comments than code. I suggest that the next time we see one of these PRs getting very bike sheddy, the (3 most) interested parties arrange for some sort of real-time collaboration (google docs / hackpad / whatever) + voice and hash it out.

If sphinx passes, I intend to merge this.

Thanks everyone!

backported to v2.x as 2c7be7c