Skip to content

Minor docstring updates on binning related plot functions #10831

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 1 commit into from
Mar 21, 2018

Conversation

timhoffm
Copy link
Member

PR Summary

As part of #10148: Docstring update for Axes.hexbin, Axes.hist and Axes.hist2d.

@@ -4193,7 +4193,7 @@ def hexbin(self, x, y, C=None, gridsize=100, bins=None,
If *C* is specified, it specifies values at the coordinate
(x[i],y[i]). These values are accumulated for each hexagonal
bin and then reduced according to *reduce_C_function*, which
defaults to numpy's mean function (np.mean). (If *C* is
defaults to numpy's mean function (`numpy.mean`). (If *C* is
Copy link
Contributor

@anntzer anntzer Mar 18, 2018

Choose a reason for hiding this comment

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

rephrase as

defaults to `numpy.mean`.

? (more concise but just as clear)

@afvincent
Copy link
Contributor

I wonder if that belongs to the scope of this PR, but I think that the description used for the bins parameter in ax.hist may be outdated. If I am correct, the related keyword argument will be passed to numpy.histogram, which means that any string among {"auto", "fd", "doane", "scott", "rice", "sturges", "sqrt"} should be valid provided that the Numpy version is recent enough, right? Currently, it looks like only "auto" is mentioned.

For example, the following (inspired from the numpy docs) runs fine on my laptop with Matplotlib 2.1.0 and numpy 1.13 (both from conda)

import numpy as np
import matplotlib.pyplot as plt

prng = np.random.RandomState(10)  # deterministic random data
a = np.hstack((prng.normal(size=1000),
               prng.normal(loc=5, scale=2, size=1000)))

non_auto_bins_str = ("fd", "doane", "scott", "rice", "sturges", "sqrt")

fig, axs = plt.subplots(nrows=2, ncols=3, sharex=True)
for ax, bins in zip(axs.flat, non_auto_bins_str):
    ax.hist(a, bins=bins)  # arguments are passed to np.histogram
    ax.set_title("'{}' bins".format(bins))

fig.tight_layout()
fig.show()

figure_1

@tacaswell
Copy link
Member

I am hesitant to document the auto-binning on histogram more than 'with a new enough version of numpy strings will work, see np.histogram' until our minimum version of numpy is high enough that we are sure it will work.

@tacaswell tacaswell added this to the v3.0 milestone Mar 18, 2018
@jklymak
Copy link
Member

jklymak commented Mar 18, 2018

How about: 'with a new enough version of numpy, strings that specify a binning strategy are also accepted, see np.histogram'.

@anntzer
Copy link
Contributor

anntzer commented Mar 18, 2018

fwiw that's numpy 1.11.

@timhoffm
Copy link
Member Author

Thanks for the hint on the binning strategy. I've now written.

With Numpy 1.11 or newer, you can alternatively provide a string describing a binning strategy, such as ‘auto’, ‘sturges’, ‘fd’, ‘doane’, ‘scott’, ‘rice’, ‘sturges’ or ‘sqrt’, see numpy.histogram.

I think that's clear and useful, and without the danger that it will not work unexpectedly.

defaults to numpy's mean function (np.mean). (If *C* is
specified, it must also be a 1-D sequence of the same length
as *x* and *y*.)
defaults to `numpy.mean`. (If *C* is specified, it must also
Copy link
Contributor

Choose a reason for hiding this comment

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

two lines earlier: space after comma

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

Input values

bins: [None | int | [int, int] | array_like | [array, array]]
bins : None or int or [int, int] or array_like or [array, array]
Copy link
Contributor

Choose a reason for hiding this comment

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

(int, int) or (array, array)? (feels like tuples, nor arrays)

Copy link
Member Author

Choose a reason for hiding this comment

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

This description came from
numpy.histogram2d. I would leave it like this.

@anntzer anntzer merged commit 197bb59 into matplotlib:master Mar 21, 2018
@timhoffm timhoffm deleted the doc-binned branch March 21, 2018 23:54
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