DOC: update the pandas.DataFrame.hist docstring #20113

DZPM · 2018-03-10T11:57:44Z

PR title is "DOC: update the pandas.DataFrame.hist docstring"
The validation script passes: scripts/validate_docstrings.py pandas.DataFrame.hist
The PEP8 style check passes: git diff upstream/master -u -- "*.py" | flake8 --diff
The html version looks good: python doc/make.py --single pandas.DataFrame.hist
It has been proofread on language by another sprint participant

################################################################################
###################### Docstring (pandas.DataFrame.hist)  ######################
################################################################################

Draw histogram of the DataFrame's series using matplotlib / pylab.

A histogram is a representation of the distribution of numerical data.
This function wraps the matplotlib histogram function for each serie in
the DataFrame. It returns an array with a plot for each histogram.

Parameters
----------
data : DataFrame
    The pandas object holding the data.
column : string or sequence
    If passed, will be used to limit data to a subset of columns.
by : object, optional
    If passed, then used to form histograms for separate groups.
grid : boolean, default True
    Whether to show axis grid lines.
xlabelsize : int, default None
    If specified changes the x-axis label size.
xrot : float, default None
    Rotation of x axis labels.
ylabelsize : int, default None
    If specified changes the y-axis label size.
yrot : float, default None
    Rotation of y axis labels.
ax : Matplotlib axes object, default None
    The histogram axes.
sharex : boolean, default True if ax is None else False
    In case subplots=True, share x axis and set some x axis labels to
    invisible; defaults to True if ax is None otherwise False if an ax
    is passed in; Be aware, that passing in both an ax and sharex=True
    will alter all x axis labels for all subplots in a figure!.
sharey : boolean, default False
    In case subplots=True, share y axis and set some y axis labels to
    invisible.
figsize : tuple
    The size of the figure to create in inches by default.
layout : tuple, optional
    Tuple of (rows, columns) for the layout of the histograms.
bins : integer or sequence, default 10
    Number of histogram bins to be used. If an integer is given, bins + 1
    bin edges are calculated and returned. If bins is a sequence, gives
    bin edges, including left edge of first bin and right edge of last
    bin. In this case, bins is returned unmodified.
kwds : Keyword Arguments
    All other plotting keyword arguments to be passed to
    matplotlib's boxplot function.

Returns
-------
axes : matplotlib.AxesSubplot or np.array of them

See Also
--------
matplotlib.axes.Axes.hist : Plot a histogram using matplotlib.

Examples
--------

.. plot::
    :context: close-figs

    >>> df = pd.DataFrame({
    ...     'length': [ 1.5, 0.5, 1.2, 0.9, 3],
    ...     'width': [ 0.7, 0.2, 0.15, 0.2,  1.1]
    ...     }, index= ['pig', 'rabbit', 'duck', 'chicken', 'horse'])
    >>> hist = df.hist(bins=3)

################################################################################
################################## Validation ##################################
################################################################################

Docstring for "pandas.DataFrame.hist" correct. :)

TomAugspurger · 2018-03-10T12:02:07Z

pandas/plotting/_core.py

@@ -2130,48 +2130,70 @@ def hist_frame(data, column=None, by=None, grid=True, xlabelsize=None,
    """
    Draw histogram of the DataFrame's series using matplotlib / pylab.


Can you remove the / pylab. It's been deprecated for ~5 years now :)

TomAugspurger · 2018-03-10T12:03:11Z

pandas/plotting/_core.py

@@ -2130,48 +2130,70 @@ def hist_frame(data, column=None, by=None, grid=True, xlabelsize=None,
    """
    Draw histogram of the DataFrame's series using matplotlib / pylab.

+    A histogram is a representation of the distribution of numerical data.


Strike numerical as hist can handle non-numeric data as well.

TomAugspurger · 2018-03-10T12:03:19Z

pandas/plotting/_core.py

@@ -2130,48 +2130,70 @@ def hist_frame(data, column=None, by=None, grid=True, xlabelsize=None,
    """
    Draw histogram of the DataFrame's series using matplotlib / pylab.

+    A histogram is a representation of the distribution of numerical data.
+    This function wraps the matplotlib histogram function for each serie in


serie -> series.

TomAugspurger · 2018-03-10T12:04:11Z

pandas/plotting/_core.py

-    ax : matplotlib axes object, default None
+        Rotation of y axis labels.
+    ax : Matplotlib axes object, default None
+        The histogram axes.


"The axes to plot the histogram on."

TomAugspurger · 2018-03-10T12:04:53Z

pandas/plotting/_core.py

    sharex : boolean, default True if ax is None else False
        In case subplots=True, share x axis and set some x axis labels to
        invisible; defaults to True if ax is None otherwise False if an ax
        is passed in; Be aware, that passing in both an ax and sharex=True
-        will alter all x axis labels for all subplots in a figure!
+        will alter all x axis labels for all subplots in a figure!.


Can remove the . since the period already ends in a punctuation character. Did the script complain about ending with a !?

Yes, the validator script complains:

Errors in parameters section Parameter "sharex" description should finish with "."

I'm rewriting it to remove the !.

TomAugspurger · 2018-03-10T12:06:16Z

pandas/plotting/_core.py

    figsize : tuple
-        The size of the figure to create in inches by default
+        The size of the figure to create in inches by default.


How about

The size in inches of the figure to create. Uses the value in `matplotlib.rcParams` by default.

TomAugspurger · 2018-03-10T12:06:54Z

pandas/plotting/_core.py

    bins : integer or sequence, default 10
        Number of histogram bins to be used. If an integer is given, bins + 1
        bin edges are calculated and returned. If bins is a sequence, gives
        bin edges, including left edge of first bin and right edge of last
        bin. In this case, bins is returned unmodified.
-    `**kwds` : other plotting keyword arguments
-        To be passed to hist function
+    kwds : Keyword Arguments


I may be wrong, but I think we're just doing these as

kwds: All other plotting ...

IOW no type.

That raises some errors in the validator:

Errors in parameters section Parameters {'kwds'} not documented Unknown parameters {'kwds :'} Parameter "kwds :" has no type

Write kwds : optional.

The validation script is wrong @datapythonista team is fixing it from London. Use always **kwargs as parameter.

TomAugspurger · 2018-03-10T12:07:30Z

pandas/plotting/_core.py

+
+    Returns
+    -------
+    axes : matplotlib.AxesSubplot or np.array of them


np.array -> numpy.ndarray

(np.array is the function, np.ndarray is the object).

DZPM · 2018-03-10T12:30:04Z

Fixed all isues (except the kwargs, as it'd fail the validation), and ammended the commit.

Thanks for the review!

dukebody · 2018-03-10T12:49:37Z

pandas/plotting/_core.py

@@ -2128,50 +2128,74 @@ def hist_frame(data, column=None, by=None, grid=True, xlabelsize=None,
               xrot=None, ylabelsize=None, yrot=None, ax=None, sharex=False,
               sharey=False, figsize=None, layout=None, bins=10, **kwds):
    """
-    Draw histogram of the DataFrame's series using matplotlib / pylab.
+    Draw histogram of the DataFrame's Series using matplotlib.


Perhaps this is written like this in other parts of the docs, but I personally find it easier to understand to say something like Draw histogram of the DataFrame's columns, instead of DataFrame's Series.

dukebody · 2018-03-10T12:51:27Z

pandas/plotting/_core.py

    xrot : float, default None
-        rotation of x axis labels
+        Rotation of x axis labels.


Can we add some explanation like "For example, a value of 90 displays the x labels rotated 90º clockwise".

dukebody · 2018-03-10T12:51:52Z

pandas/plotting/_core.py

    yrot : float, default None
-        rotation of y axis labels
-    ax : matplotlib axes object, default None
+        Rotation of y axis labels.


Same here, make clear what a value of 90 means, for example.

dukebody · 2018-03-10T12:52:29Z

pandas/plotting/_core.py

-        is passed in; Be aware, that passing in both an ax and sharex=True
-        will alter all x axis labels for all subplots in a figure!
+        is passed in.
+        Be aware: passing in both an ax and sharex=True will alter all x axis


I'd say "Note" instead of "Be aware", which feels a bit more agressive.

dukebody · 2018-03-10T12:53:14Z

pandas/plotting/_core.py

-        To be passed to hist function
+    kwds : Keyword Arguments
+        All other plotting keyword arguments to be passed to
+        matplotlib's boxplot function.


Link to this function using :method: or :class:.

dukebody · 2018-03-10T12:54:12Z

pandas/plotting/_core.py

+        :context: close-figs
+
+        >>> df = pd.DataFrame({
+        ...     'length': [ 1.5, 0.5, 1.2, 0.9, 3],


Mind the spacing: [ 1.5, .... In next lines, there are two spaces between the 0.2 and the 1.1.

dukebody · 2018-03-10T12:54:33Z

pandas/plotting/_core.py

+    .. plot::
+        :context: close-figs
+
+        >>> df = pd.DataFrame({


Can you write some introduction explaining in words what the code does?

dukebody · 2018-03-10T12:57:35Z

pandas/plotting/_core.py

+        ...     'length': [ 1.5, 0.5, 1.2, 0.9, 3],
+        ...     'width': [ 0.7, 0.2, 0.15, 0.2,  1.1]
+        ...     }, index= ['pig', 'rabbit', 'duck', 'chicken', 'horse'])
+        >>> hist = df.hist(bins=3)


There are a lot of arguments for this method. Can you write a couple more examples trying to show how do they behave?

jreback · 2018-03-10T14:45:45Z

pandas/plotting/_core.py

+
+    A histogram is a representation of the distribution of data.
+    This function wraps the matplotlib histogram function for each series in
+    the DataFrame. It returns an array with a plot for each histogram.


can you link to wikipedia (here or in Notes is ok)

dukebody · 2018-03-10T18:58:02Z

pandas/plotting/_core.py

+
+    A histogram is a representation of the distribution of data.
+    This function wraps the matplotlib histogram function for each series in
+    the DataFrame. It returns an array with a plot for each histogram.


"It returns an array with a plot for each histogram" is a bit misleading. What about "A histogram is plotted for every column of the DataFrame"?

dukebody · 2018-03-10T18:59:13Z

pandas/plotting/_core.py

@@ -2128,50 +2128,79 @@ def hist_frame(data, column=None, by=None, grid=True, xlabelsize=None,
               xrot=None, ylabelsize=None, yrot=None, ax=None, sharex=False,
               sharey=False, figsize=None, layout=None, bins=10, **kwds):
    """
-    Draw histogram of the DataFrame's series using matplotlib / pylab.
+    Draw histogram of the DataFrame's columns using matplotlib.


As in other cases, I believe that we should delay the fact that it is using matplotlib under the hood to the extended description and get it out of the short one.

codecov · 2018-03-13T20:40:57Z

Codecov Report

Merging #20113 into master will decrease coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #20113      +/-   ##
==========================================
- Coverage   91.72%    91.7%   -0.03%     
==========================================
  Files         150      150              
  Lines       49156    49156              
==========================================
- Hits        45090    45078      -12     
- Misses       4066     4078      +12

Flag	Coverage Δ
#multiple	`90.08% <ø> (-0.03%)`	⬇️
#single	`41.85% <ø> (ø)`	⬆️

Impacted Files	Coverage Δ
pandas/plotting/_core.py	`82.23% <ø> (ø)`	⬆️
pandas/plotting/_converter.py	`65.07% <0%> (-1.74%)`	⬇️
pandas/core/generic.py	`95.84% <0%> (ø)`	⬆️
pandas/core/indexes/multi.py	`95.06% <0%> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 840d432...ab1e17d. Read the comment docs.

TomAugspurger · 2018-03-13T20:41:08Z

Thanks @DZPM !

TomAugspurger reviewed Mar 10, 2018

View reviewed changes

DZPM force-pushed the docstring_dataframe_hist branch from 8898b45 to d7a1ecd Compare March 10, 2018 12:27

DZPM force-pushed the docstring_dataframe_hist branch from d7a1ecd to 8f5c449 Compare March 10, 2018 12:56

dukebody reviewed Mar 10, 2018

View reviewed changes

DOC: Improve dosctring for pandas.DataFrame.hist

8995dc3

DZPM force-pushed the docstring_dataframe_hist branch from 8f5c449 to 8995dc3 Compare March 10, 2018 13:36

jonas-schulze mentioned this pull request Mar 10, 2018

DOC: Improve pandas.Series.plot.kde docstring and kwargs rewording for whole file #20041

Merged

jreback added Docs Visualization plotting labels Mar 10, 2018

jreback requested changes Mar 10, 2018

View reviewed changes

dukebody reviewed Mar 10, 2018

View reviewed changes

Updated

ab1e17d

TomAugspurger added this to the 0.23.0 milestone Mar 13, 2018

TomAugspurger merged commit ffe297c into pandas-dev:master Mar 13, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DOC: update the pandas.DataFrame.hist docstring #20113

DOC: update the pandas.DataFrame.hist docstring #20113

DZPM commented Mar 10, 2018

TomAugspurger Mar 10, 2018

TomAugspurger Mar 10, 2018

TomAugspurger Mar 10, 2018

TomAugspurger Mar 10, 2018

TomAugspurger Mar 10, 2018

DZPM Mar 10, 2018

TomAugspurger Mar 10, 2018

TomAugspurger Mar 10, 2018

DZPM Mar 10, 2018

dukebody Mar 10, 2018

DZPM Mar 10, 2018

TomAugspurger Mar 10, 2018

DZPM commented Mar 10, 2018

dukebody Mar 10, 2018

dukebody Mar 10, 2018

dukebody Mar 10, 2018

dukebody Mar 10, 2018

dukebody Mar 10, 2018

dukebody Mar 10, 2018

dukebody Mar 10, 2018

dukebody Mar 10, 2018

jreback Mar 10, 2018

dukebody Mar 10, 2018

dukebody Mar 10, 2018

codecov bot commented Mar 13, 2018

TomAugspurger commented Mar 13, 2018

		@@ -2130,48 +2130,70 @@ def hist_frame(data, column=None, by=None, grid=True, xlabelsize=None,
		"""
		Draw histogram of the DataFrame's series using matplotlib / pylab.

DOC: update the pandas.DataFrame.hist docstring #20113

DOC: update the pandas.DataFrame.hist docstring #20113

Conversation

DZPM commented Mar 10, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DZPM commented Mar 10, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Mar 13, 2018

Codecov Report

TomAugspurger commented Mar 13, 2018