-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
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
Conversation
lib/matplotlib/colors.py
Outdated
then v is mapped to color j; | ||
Notes | ||
----- | ||
If :code:`b[i] <= v < b[i+1]` then v is mapped to color j; |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
lib/matplotlib/colors.py
Outdated
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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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").
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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.
lib/matplotlib/colors.py
Outdated
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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
lib/matplotlib/colors.py
Outdated
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 |
There was a problem hiding this comment.
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
There was a problem hiding this 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)
lib/matplotlib/colors.py
Outdated
b[i] <= v < b[i+1] | ||
Parameters | ||
---------- | ||
boundaries : sequence |
There was a problem hiding this comment.
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
lib/matplotlib/colors.py
Outdated
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 |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Those should be fixed now |
lib/matplotlib/colors.py
Outdated
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"interpolation"
Thanks! |
invert
will always raise aValueError