MEP10 - refactored hlines and vlines documentation #1795

Merged
merged 4 commits into from Mar 14, 2013

Conversation

Projects
None yet
3 participants
Contributor

NelleV commented Feb 28, 2013

Refactored hlines and vlines documentation to match MEP10

Thanks,
N

@pelson pelson and 2 others commented on an outdated diff Feb 28, 2013

lib/matplotlib/axes.py
- %(LineCollection)s
@pelson

pelson Feb 28, 2013

Member

Have the contents of this now been included directly?

@NelleV

NelleV Feb 28, 2013

Contributor

Nope, this is lost. Using this trick doesn't work (it is not rendered properly), and numpydoc has no way to do introspection to spot that these are linecollection kwargs.

@mdboom

mdboom Feb 28, 2013

Owner

I know that whether to continue to include these inline was controversial -- but I tend to fall on the side of not duplicating this information everywhere. If we do that, though, we need a clear link to the LineProperties class, and my suggested change for the Returns section above does that.

@mdboom mdboom commented on an outdated diff Feb 28, 2013

lib/matplotlib/axes.py
- kwargs are :class:`~matplotlib.collections.LineCollection` properties:
+ Returns
+ -------
+ matplotlib.collections.LineCollection
@mdboom

mdboom Feb 28, 2013

Owner

This has to be of the form name : type or it doesn't render correctly, i.e.

lines : matplotlib.collections.LineCollection

@mdboom mdboom commented on an outdated diff Feb 28, 2013

lib/matplotlib/axes.py
- kwargs are :class:`~matplotlib.collections.LineCollection` properties:
+ Returns
+ -------
+ matplotlib.collections.LineCollection
+
+ Other parameters
+ ----------------
+ kwargs correspond to matplotlib.collections.LineCollection properties.
+
+ Notes
+ -----
+ .. plot:: mpl_examples/pylab_examples/vline_hline_demo.py
@mdboom

mdboom Feb 28, 2013

Owner

Notes needs to come after See also (again, it doesn't render correctly).

@mdboom mdboom and 1 other commented on an outdated diff Feb 28, 2013

lib/matplotlib/axes.py
- kwargs are :class:`~matplotlib.collections.LineCollection` properties:
+ Returns
+ -------
+ matplotlib.collections.LineCollection
+
+ Other parameters
+ ----------------
+ kwargs correspond to matplotlib.collections.LineCollection properties.
@mdboom

mdboom Feb 28, 2013

Owner

This doesn't render correctly, plus it would be nice if it linked to the LineCollection class:

kwargs : `~matplotlib.collections.LineCollection` properties
@NelleV

NelleV Feb 28, 2013

Contributor

Maybe this should fall in the Notes section ?

@mdboom

mdboom Feb 28, 2013

Owner

Well, they are "Other Parameters" -- it sort of makes sense there, but it does mean we have to use numpydoc's parameter syntax here. Saying "kwargs" as a parameter name is a bit clunky/obscure, though, admittedly.

@NelleV

NelleV Feb 28, 2013

Contributor

It's fixed !

Owner

mdboom commented Feb 28, 2013

This looks good -- this is the first time I've spent the time to actually build and look at the output of your MEP10 changes. It looks great -- but sorry if I'm commenting on issues that may have already gone a different way in earlier PRs (such as how the Returns section is written etc.)

Contributor

NelleV commented Feb 28, 2013

For the curious, I just set up an automatic build of the documentation of the master branch of the repository here:
http://cbio.ensmp.fr/~nvaroquaux/matplotlib/doc/ and a check of the flake8 report here (but links don't work):
http://cbio.ensmp.fr/~nvaroquaux/matplotlib/flake8_report/

The flake8 report could be done by shining panda, or something more adapted than bash scripts.

Owner

mdboom commented Feb 28, 2013

Thanks. Once we get funding in place, I hope to set up a Shining Panda pro account for matplotlib soon -- and this is definitely the kind of useful thing to do over there.

Owner

mdboom commented Feb 28, 2013

Great. I think it's good (as we doing here) to get the first few docstrings really right and then running through the rest should involve a lot of just following the same pattern.

Contributor

NelleV commented Mar 7, 2013

I think this is ready to be merged.

Owner

mdboom commented Mar 7, 2013

I'm seeing funky formatting when building this. It looks like "Other Parameters" is divided into two sections...?
funky-formatting

Contributor

NelleV commented Mar 7, 2013

Other parameters can have a list of parameter as the "parameters" section has, so I think this is normal.

Owner

mdboom commented Mar 7, 2013

But why is it on a different background color, with a line above it (unlike Parameters above)? It looks like it's in a different section (at least to my eyes).

Contributor

NelleV commented Mar 14, 2013

It is either the way it is supposed to be or a bug in numpydoc. After digging in numpy and scipy's code, I finnaly found a place where this was used:http://docs.scipy.org/doc/scipy/reference/generated/scipy.weave.inline.html#scipy.weave.inline

There's nothing much we can do about that, except submit a patch to numpydoc.

Owner

mdboom commented Mar 14, 2013

Ok thanks for looking into it. Since it's not anything we can fix from our end, I'll go ahead and merge this. Thanks.

@mdboom mdboom added a commit that referenced this pull request Mar 14, 2013

@mdboom mdboom Merge pull request #1795 from NelleV/MEP10_hvlines
MEP10 - refactored hlines and vlines documentation
9690232

@mdboom mdboom merged commit 9690232 into matplotlib:master Mar 14, 2013

1 check passed

default The Travis build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment