Skip to content

Clean up BoundaryNorm docstring #8007

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Mar 5, 2017

Conversation

dstansby
Copy link
Member

@dstansby dstansby commented Feb 1, 2017

  • Make numpydoc compatible
  • Add note that calling invert will always raise a ValueError

then v is mapped to color j;
Notes
-----
If :code:`b[i] <= v < b[i+1]` then v is mapped to color j;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if the two lengths don't match?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use double backquotes here and below.

Copy link
Member Author

@dstansby dstansby Feb 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For clip or for True and 0?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should the floor function be in the math directive?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually this is about to go in my next revision anyway

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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

@NelleV NelleV left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

b[i] <= v < b[i+1]
Parameters
----------
boundaries : sequence
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be array-like

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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@NelleV NelleV changed the title Clean up BoundaryNorm docstring [MRG+1] Clean up BoundaryNorm docstring Feb 24, 2017
@dstansby
Copy link
Member Author

Those should be fixed now

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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"interpolation"

@anntzer
Copy link
Contributor

anntzer commented Mar 5, 2017

Thanks!

@anntzer anntzer merged commit 3bfa438 into matplotlib:master Mar 5, 2017
@QuLogic QuLogic changed the title [MRG+1] Clean up BoundaryNorm docstring Clean up BoundaryNorm docstring Mar 6, 2017
@QuLogic QuLogic added this to the 2.1 (next point release) milestone Mar 6, 2017
@dstansby dstansby deleted the boundarynorm-docstring branch April 2, 2017 16:44
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.

5 participants