[MRG+1] [DOC] Turn ginput dostring into a numpydocstring #8172

Merged
merged 1 commit into from Mar 8, 2017

Conversation

Projects
None yet
6 participants
Contributor

dstansby commented Feb 28, 2017

No description provided.

lib/matplotlib/figure.py
+ ----------
+ n : int, optional
+ Number of mouse clicks to accumulate. If negative, accumulate
+ clicks until the input is terminated manually.
@afvincent

afvincent Feb 28, 2017

Contributor

Add "Default is 1."?

lib/matplotlib/figure.py
- (or potentially both mouse buttons at once) terminates the input.
-
- Right clicking cancels last input.
+ Blocking call to interact with a figure. This will wait for *n*
@anntzer

anntzer Feb 28, 2017

Contributor

I would rewrite the paragraph as

Wait until the user clicks n times on the figure. Returns the coordinates of each click in a list.

@NelleV

NelleV Mar 2, 2017

Contributor

Also, I believe (though it is very unsure) that we decided to drop the "*", which are a remainer of the past in Matplotlib's documentation.

@NelleV

I think this is a great improvement :)

lib/matplotlib/figure.py
- (or potentially both mouse buttons at once) terminates the input.
-
- Right clicking cancels last input.
+ Blocking call to interact with a figure. This will wait for *n*
@NelleV

NelleV Mar 2, 2017

Contributor

Also, I believe (though it is very unsure) that we decided to drop the "*", which are a remainer of the past in Matplotlib's documentation.

lib/matplotlib/figure.py
+ ----------
+ n : int, optional
+ Number of mouse clicks to accumulate. If negative, accumulate
+ clicks until the input is terminated manually. Default is 1.
@NelleV

NelleV Mar 2, 2017

Contributor

the numpydoc formats puts the default on the first line:

n : int, optional, default: 1

I find the numpydoc "offical" version more readable.

lib/matplotlib/figure.py
+ Returns
+ -------
+ points : list of tuples
+ The clicked points. ``len(points)`` will equal the number of points
@anntzer

anntzer Mar 2, 2017

Contributor

I would rewrite the entire thing (the three sentences) as A list of (x, y) coordinates of the clicks. There is no "number of requested points" if n<0, and there can be fewer points if the user presses mouse_stop prematurely.

@dstansby

dstansby Mar 2, 2017

Contributor

Yep, sounds better.

@QuLogic

LGTM modulo one item.

lib/matplotlib/figure.py
- (or potentially both mouse buttons at once) terminates the input.
-
- Right clicking cancels last input.
+ Blocking call to interact with a figure. Wait until the user clicks *n*
@QuLogic

QuLogic Mar 2, 2017

Member

The first line should be by itself with the remaining description on another paragraph (one blank line later).

lib/matplotlib/figure.py
+ Number of seconds to wait before timing out. If zero or negative
+ will never timeout.
+ show_clicks : bool, optional, default: False
+ If True, show a red cross at the location of each click
@anntzer

anntzer Mar 2, 2017

Contributor

Add final dots here and below (including returns), and squash? (otherwise that's a lot of commits :-))

@phobson

phobson Mar 3, 2017

Member

In addition to the comment above, I think the format should be, for example

show_clicks : bool, optional (default = False)
@QuLogic

QuLogic Mar 3, 2017

Member

That format seems much rarer in our codebase than the existing format.

@anntzer

anntzer Mar 5, 2017

Contributor

@QuLogic I will approve the PR after this (the dots -- squashing is up to you) is fixed.

@anntzer

anntzer Mar 6, 2017

Contributor

The format for defaults should probably added to the documenting matplotlib developer's guide. Note that numpy itself gives the example

Description of parameter `x` (the default is -1, which implies summation
over all axes).

(which is different and I dislike)

@QuLogic

QuLogic Mar 6, 2017

Member

I think you meant to tag @dstansby there; I'm fine with this when you are.

lib/matplotlib/figure.py
+ Number of seconds to wait before timing out. If zero or negative
+ will never timeout.
+ show_clicks : bool, optional, default: False
+ If True, show a red cross at the location of each click
@anntzer

anntzer Mar 5, 2017

Contributor

@QuLogic I will approve the PR after this (the dots -- squashing is up to you) is fixed.

dstansby changed the title from Turn ginput dostring into a numpydocstring to [DOC] Turn ginput dostring into a numpydocstring Mar 7, 2017

@dstansby dstansby Make ginput dostring into a numpydocstring
Small fixes

Add default value for npoints

Suggested changes to ginput docstring

Better description of ginput return

Put summary line by itself at the top

Add in missing full stops
0151c05
Contributor

dstansby commented Mar 7, 2017

I've added in the extra full stops, and given it a squash.

@anntzer

anntzer approved these changes Mar 7, 2017

anntzer changed the title from [DOC] Turn ginput dostring into a numpydocstring to [MRG+1] [DOC] Turn ginput dostring into a numpydocstring Mar 7, 2017

@NelleV NelleV merged commit 8277ddf into matplotlib:master Mar 8, 2017

3 of 5 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
codecov/patch Coverage not affected when comparing 1d45c08...0151c05
Details
codecov/project/library 57.82% (+0.24%) compared to 1d45c08
Details
codecov/project/tests 98.23% (target 97.9%)
Details
Contributor

NelleV commented Mar 8, 2017

Thanks @dstansby !

dstansby deleted the dstansby:ginput-docstring branch Apr 2, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment