Clean up BoundaryNorm docstring #8007

Merged
merged 6 commits into from Mar 5, 2017

Conversation

Projects
5 participants
Contributor

dstansby commented Feb 1, 2017

  • Make numpydoc compatible
  • Add note that calling invert will always raise a ValueError
@dstansby dstansby Clean up BoundaryNorm docstring
2b6699c
lib/matplotlib/colors.py
- then v is mapped to color j;
+ Notes
+ -----
+ If :code:`b[i] <= v < b[i+1]` then v is mapped to color j;
@anntzer

anntzer Feb 1, 2017

Contributor

What happens if the two lengths don't match?

@dstansby

dstansby Feb 2, 2017

Contributor

I have no idea, will try and work out and update this later today.

lib/matplotlib/colors.py
+ ncolors : int
+ Number of colors in the colormap to be used
+ clip : bool, optional
+ If clip is *True*, out of range values are mapped to *0* if they
@anntzer

anntzer Feb 1, 2017

Contributor

Use double backquotes here and below.

@dstansby

dstansby Feb 2, 2017 edited

Contributor

For clip or for True and 0?

@anntzer

anntzer Feb 2, 2017

Contributor

I would say for all (they're all "code").

@dstansby

dstansby Feb 2, 2017

Contributor

But http://matplotlib.org/devdocs/devel/documenting_mpl.html#formatting says to explicitly not do that! (at least for arguments to functions)

@anntzer

anntzer Feb 2, 2017

Contributor

Indeed, that simply means I've been doing things wrong :-)
https://docs.python.org/devguide/documenting.html#id3 (the mpl docs refer to consistency with python itself) says to use emphasis specifically for local variables (i.e. arguments) only.

@dstansby dstansby Document what happens when ncolors > bins
32641c7
Contributor

anntzer commented on lib/matplotlib/colors.py in 32641c7 Feb 2, 2017

I think this obscures the most common case where both are of equal length. I would say something like (could be enhanced) "Boundaries define bins, that are each mapped to the color with the same index (or linearly interpolating the index if the sizes don't match)."

lib/matplotlib/colors.py
+ Notes
+ -----
+ If :code:`b[i] <= v < b[i+1]` then v is mapped to
+ floor( :math:`\frac{n_{colors}- 1}{n_{bins} - 1} * i`), where nbins is
@phobson

phobson Feb 2, 2017

Member

should the floor function be in the math directive?

@dstansby

dstansby Feb 2, 2017

Contributor

I thought it looked better visually outside, but I can put it inside.

@dstansby

dstansby Feb 2, 2017

Contributor

Actually this is about to go in my next revision anyway

@dstansby dstansby BoundaryNorm docstring clarification
b3b0750
lib/matplotlib/colors.py
- above *boundaries[-1]*.
+ If *clip* is ``True``, out of range values are mapped to 0 if they
+ are below ``boundaries[0]`` or mapped to *ncolors* - 1 if they are
+ above ``boundaries[-1]``.
If clip is *False*, out of range values are mapped to *-1* if they
@phobson

phobson Feb 3, 2017

Member

False and boundaries[0] to be consistent with the previous paragrpha

@dstansby dstansby BoundaryNorm docstring formatting
da208dd
@NelleV

NelleV approved these changes Feb 24, 2017

LGTM 👍 apart from my minor nitpicks (which I am happy to fix before merging)

lib/matplotlib/colors.py
- b[i] <= v < b[i+1]
+ Parameters
+ ----------
+ boundaries : sequence
@NelleV

NelleV Feb 24, 2017

Contributor

this should be array-like

lib/matplotlib/colors.py
+ ncolors : int
+ Number of colors in the colormap to be used
+ clip : bool, optional
+ If *clip* is ``True``, out of range values are mapped to 0 if they
@NelleV

NelleV Feb 24, 2017

Contributor

Can you please remove the *? They make docstrings hard to read in the terminal

+ Raises
+ ------
+ ValueError
+ BoundaryNorm is not invertible, so calling this method will always
@NelleV

NelleV Feb 24, 2017

Contributor

👍

NelleV changed the title from Clean up BoundaryNorm docstring to [MRG+1] Clean up BoundaryNorm docstring Feb 24, 2017

@dstansby dstansby Small docstring fixes
99279b4
Contributor

dstansby commented Feb 25, 2017

Those should be fixed now

lib/matplotlib/colors.py
- If clip == True, out-of-range values
- are mapped to 0 if low and ncolors-1 if high.
+ If the number of bins doesn't equal *ncolors*, the color is chosen
+ by linear inpolation of the bin number onto color numbers.
@anntzer

anntzer Feb 25, 2017

Contributor

"interpolation"

@dstansby dstansby Fix small spelling mistake
9dd7110
Contributor

anntzer commented Mar 5, 2017

Thanks!

@anntzer anntzer merged commit 3bfa438 into matplotlib:master Mar 5, 2017

1 of 2 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

QuLogic changed the title from [MRG+1] Clean up BoundaryNorm docstring to Clean up BoundaryNorm docstring Mar 6, 2017

QuLogic added this to the 2.1 (next point release) milestone Mar 6, 2017

QuLogic removed the needs_review label Mar 23, 2017

QuLogic moved from Needs review to Done in Reviewing pull requests. Mar 23, 2017

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

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