DOC: update the pandas.DataFrame.plot.area docstring #20170

DZPM · 2018-03-10T15:04:41Z

PR title is "DOC: update the pandas.DataFrame.plot.area docstring"
The validation script passes: scripts/validate_docstrings.py pandas.DataFrame.plot.area
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.plot.area
It has been proofread on language by another sprint participant

################################################################################
#################### Docstring (pandas.DataFrame.plot.area) ####################
################################################################################

Draw an area plot of the input series using matplotlib.

An area plot displays graphically quantitative data.
This function wraps the matplotlib area function.

Parameters
----------
x : label or position, optional
    Coordinates for X point.
y : label or position, optional
    Coordinates for Y point.
kwds : optional
    Keyword arguments to pass on to :py:meth:`pandas.DataFrame.plot`.

Returns
-------
matplotlib.AxesSubplot or numpy.ndarray
    Area plot generated.

See Also
--------
pandas.DataFrame.plot : Draw a plot using matplotlib.

Examples
--------
.. plot::
    :context: close-figs

    This example draws an area plot based on the length and width of
    some animals, displayed in three bins

    >>> df = pd.DataFrame({
    ...     'sales': [3, 2, 3, 9, 10, 6],
    ...     'visits': [20, 42, 28, 62, 81, 50],
    ... })
    >>> area = df.plot.area()

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

Docstring for "pandas.DataFrame.plot.area" correct. :)

kynan · 2018-03-10T15:32:49Z

pandas/plotting/_core.py

-        Area plot
+        Draw an area plot of the input series using matplotlib.
+
+        An area plot displays graphically quantitative data.


displays quantitative data visually

kynan · 2018-03-10T15:33:43Z

pandas/plotting/_core.py

-            Coordinates for each point.
-        `**kwds` : optional
+        x : label or position, optional
+            Coordinates for X point.


kynan · 2018-03-10T15:33:49Z

pandas/plotting/_core.py

+        x : label or position, optional
+            Coordinates for X point.
+        y : label or position, optional
+            Coordinates for Y point.


jorisvandenbossche

Thanks! added some comments

jorisvandenbossche · 2018-03-10T15:36:21Z

pandas/plotting/_core.py

+
+        See Also
+        --------
+        pandas.DataFrame.plot : Draw a plot using matplotlib.


The explanation here is a bit strange, as it would seem to say that this one is not plotting with matplotlib

jorisvandenbossche · 2018-03-10T15:36:46Z

pandas/plotting/_core.py

            Keyword arguments to pass on to :py:meth:`pandas.DataFrame.plot`.

        Returns
        -------
-        axes : matplotlib.AxesSubplot or np.array of them
+        matplotlib.AxesSubplot or numpy.ndarray
+            Area plot generated.


Can you explain something about when ndarray is returned?

Yep. In this case the existing explanation Area plot generated doesn't tell a lot. How is a numpy.ndarray the area plot generated?

jorisvandenbossche · 2018-03-10T15:38:03Z

pandas/plotting/_core.py

@@ -2818,18 +2818,42 @@ def kde(self, bw_method=None, ind=None, **kwds):

    def area(self, x=None, y=None, **kwds):
        """
-        Area plot
+        Draw an area plot of the input series using matplotlib.


"input series" -> input is a bit strange wording as it is the calling object (not passed as input) + it should be DataFrame instead of Series ?

I don't know if we want to mention the underlying library this is using at this level. Does the user really care so much? In the extended description is more OK IMO.

Also, as @jorisvandenbossche says, the description doesn't make sense for dataframes. Probably what the function does is to display a stacked area plot?

dukebody · 2018-03-10T18:19:39Z

Can you post a screenshot of the rendered HTML? It will help reviewing without having to check out your branch and compiling it.

dukebody

I believe this needs some better explanations of the extended description, arguments and more examples that help to understand how this graph works.

dukebody · 2018-03-10T18:21:56Z

pandas/plotting/_core.py

@@ -2818,18 +2818,42 @@ def kde(self, bw_method=None, ind=None, **kwds):

    def area(self, x=None, y=None, **kwds):
        """
-        Area plot
+        Draw an area plot of the input series using matplotlib.


I don't know if we want to mention the underlying library this is using at this level. Does the user really care so much? In the extended description is more OK IMO.

dukebody · 2018-03-10T18:23:10Z

pandas/plotting/_core.py

@@ -2818,18 +2818,42 @@ def kde(self, bw_method=None, ind=None, **kwds):

    def area(self, x=None, y=None, **kwds):
        """
-        Area plot
+        Draw an area plot of the input series using matplotlib.


Also, as @jorisvandenbossche says, the description doesn't make sense for dataframes. Probably what the function does is to display a stacked area plot?

dukebody · 2018-03-10T18:25:34Z

pandas/plotting/_core.py

-        Area plot
+        Draw an area plot of the input series using matplotlib.
+
+        An area plot displays graphically quantitative data.


It would be great to add some info about why this function is useful or some basic use cases. Displays quantitative data visually doesn't really give me very interesting information about what is the shape or usage of the plot.

dukebody · 2018-03-10T18:27:21Z

pandas/plotting/_core.py

-        x, y : label or position, optional
-            Coordinates for each point.
-        `**kwds` : optional
+        x : label or position, optional


This should follow the guide: https://python-sprints.github.io/pandas/guide/pandas_docstring.html#section-3-parameters

So it should indicate the type of the parameter in the first line (plus optional), and later the meaning of the parameter.

dukebody · 2018-03-10T18:29:12Z

pandas/plotting/_core.py

-            Coordinates for each point.
-        `**kwds` : optional
+        x : label or position, optional
+            Coordinates for X point.


From this description of the arguments x and y I really don't understand what they change in the plot. Can we make the parameter description a bit longer explaining what do they mean?

dukebody · 2018-03-10T18:29:51Z

pandas/plotting/_core.py

+            Coordinates for X point.
+        y : label or position, optional
+            Coordinates for Y point.
+        kwds : optional


As mentioned in other PRs, you should use **kwds instead. Sorry for the great big confusion here.

Using **kwds raises errors in the current version of the validator:

Errors in parameters section Parameters {'kwds'} not documented Unknown parameters {'**kwds'}

I think it's important to be consistent between the validator and the code.

Of course, once the validator is updated, I agree this must be changed.

You can ignore that error in the validation script for now.

Ok, updated!

dukebody · 2018-03-10T18:31:31Z

pandas/plotting/_core.py

            Keyword arguments to pass on to :py:meth:`pandas.DataFrame.plot`.

        Returns
        -------
-        axes : matplotlib.AxesSubplot or np.array of them
+        matplotlib.AxesSubplot or numpy.ndarray
+            Area plot generated.


Yep. In this case the existing explanation Area plot generated doesn't tell a lot. How is a numpy.ndarray the area plot generated?

dukebody · 2018-03-10T18:34:30Z

pandas/plotting/_core.py

+        .. plot::
+            :context: close-figs
+
+            This example draws an area plot based on the length and width of


I think is better to put the text before the start of the plot block, and leave the block only for the code that constructs the plot.

The example this sentence talks about doesn't correspond to the dataframe below.

Don't write in 3rd person This example does this. Better write directly in imperative or present form, like Draw an area plot based on or We can draw an area plot based on..., but IMO mentioning this example is somewhat redundant when we are already in the examples section. :)

dukebody · 2018-03-10T18:35:24Z

pandas/plotting/_core.py

+            :context: close-figs
+
+            This example draws an area plot based on the length and width of
+            some animals, displayed in three bins


In three bins? Where does the three come from?

Yes, it's wrong, copied it from another example. Removed it.

dukebody · 2018-03-10T18:37:00Z

pandas/plotting/_core.py

+            ...     'sales': [3, 2, 3, 9, 10, 6],
+            ...     'visits': [20, 42, 28, 62, 81, 50],
+            ... })
+            >>> area = df.plot.area()


I'm not sure about this, but if the result of this method is an axis object, perhaps it's better to assign it to a variable named ax like in other examples.

Can you add other examples where the usage of the x and y parameters is shown? Also an example where a numpy.ndarray is returned.

Not sure about how to define a "usable" example, a little help would be appreciated!

dukebody · 2018-03-13T07:41:11Z

Is there anything we can do to help finishing the work on this PR?

DZPM · 2018-03-13T09:24:25Z

I'll update the PR later this week, just need some time 👍

codecov · 2018-03-15T11:22:55Z

Codecov Report

Merging #20170 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #20170   +/-   ##
=======================================
  Coverage   92.07%   92.07%           
=======================================
  Files         169      169           
  Lines       50683    50683           
=======================================
  Hits        46666    46666           
  Misses       4017     4017

Flag	Coverage Δ
#multiple	`90.48% <ø> (ø)`	⬆️
#single	`42.34% <ø> (ø)`	⬆️

Impacted Files	Coverage Δ
pandas/plotting/_core.py	`83.48% <ø> (ø)`	⬆️

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 d7b408e...3f02fe7. Read the comment docs.

DZPM · 2018-03-15T11:23:35Z

Fixed most of the suggestions, except for:

writing **kwds instead of kwds. That would break the validation.
more/better examples: I ask for inspiration :)

Also, a screenshot of the example:

jorisvandenbossche

Thanks for the updates! Added some more comments

jorisvandenbossche · 2018-03-15T12:20:55Z

pandas/plotting/_core.py

@@ -3115,19 +3115,42 @@ def kde(self, bw_method=None, ind=None, **kwds):

    def area(self, x=None, y=None, **kwds):
        """
-        Area plot
+        Draw an stacked area plot.


jorisvandenbossche · 2018-03-15T12:32:13Z

pandas/plotting/_core.py

-        x, y : label or position, optional
-            Coordinates for each point.
-        `**kwds` : optional
+        x : label or position, optional, default None


you can leave out the ", default None" part (saying it is "optional" should be informative enough").

But we should specify here what it used for x by default. I suppose this is the index?

(and the same comment for y

Yes I believe it's the index by default.

jorisvandenbossche · 2018-03-15T12:35:36Z

pandas/plotting/_core.py

+            Coordinates for X axis.
+        y : label or position, optional, default None
+            Coordinates for Y axis.
+        **kwds : optional


Can you also document the stacked keyword? I know it is not in the signature, but it is a relevant one for this type of plot, so I would add it.
See http://pandas.pydata.org/pandas-docs/stable/visualization.html#area-plot

(the validation script will complain about it, but you can ignore that again)

jorisvandenbossche · 2018-03-15T12:37:46Z

pandas/plotting/_core.py

            Additional keyword arguments are documented in
            :meth:`pandas.DataFrame.plot`.

        Returns
        -------
-        axes : :class:`matplotlib.axes.Axes` or numpy.ndarray of them
+        matplotlib.axes.Axes or numpy.ndarray
+            Area plot, or array of area plots.


It would be good to add when an array is returned, which is when subplots=True is specified

jorisvandenbossche · 2018-03-15T12:39:03Z

pandas/plotting/_core.py

+            ...     'sales': [3, 2, 3, 9, 10, 6],
+            ...     'visits': [20, 42, 28, 62, 81, 50],
+            ... })
+            >>> ax = df.plot.area()


Some ideas for the examples:

I would also show one where you use the x and y keywords

I would show one with stacked=False

I think it is typical to use area plots with time series data. So maybe it would be nice to add a DatetimeIndex to the df.

TomAugspurger · 2018-03-15T12:46:31Z

writing **kwds instead of kwds. That would break the validation.

You can ignore this, **kwargs is correct, the script is incorrect :)

TomAugspurger · 2018-03-15T12:47:32Z

http://pandas.pydata.org/pandas-docs/stable/visualization.html#area-plot has an example. The same thing with a datetimeidnex from pd.date_range would be nice (maybe a bit wider of an aspect ratio).

DZPM · 2018-03-26T20:19:46Z

Hi,

I've updated with all the proposals, and rebased.

Screenshots of the rendered example section:

It looks much better :) Thanks for the feedback!

DZPM · 2018-04-06T12:31:46Z

Ping @kynan @jorisvandenbossche @TomAugspurger @dukebody

Feel free to keep reviewing, or do the merge :)

jorisvandenbossche · 2018-04-09T08:44:29Z

@DZPM Thanks for the reminder!
Can you add a datetime index to the example frame? See #20170 (comment)

jorisvandenbossche

Added a few more comments

BTW, for future reference, if you add new commits to the branch with additional changes instead of squashing everything into a single commit, it is a bit easier to see what has been updated since the last review

jorisvandenbossche · 2018-04-09T08:48:09Z

pandas/plotting/_core.py

-            Coordinates for each point.
-        `**kwds` : optional
+        x : label or position, optional
+            Index, coordinates for X axis.


The "Index, " part is a bit confusing I think. I would reword this as : Coordinates for the Y axis. By default uses the index.

jorisvandenbossche · 2018-04-09T08:49:11Z

pandas/plotting/_core.py

+        x : label or position, optional
+            Index, coordinates for X axis.
+        y : label or position, optional
+            Index, coordinates for Y axis.


Here the default is not the index, but all columns. So maybe something like "Column to plot. By default uses all columns." ?

jorisvandenbossche · 2018-04-09T08:54:22Z

pandas/plotting/_core.py

+            ...     'visits': [20, 42, 28, 62, 81, 50],
+            ... })
+            >>> ax = df.plot.area(y='sales')
+            >>> ax = df.plot.area(y='signups')


I think showing only the first is fine to illustrate the y keyword (to save some vertical screen estate)

(if you want to have all of them on a separate figure, one could also use subplots=True)

datapythonista · 2018-07-22T00:54:28Z

@DZPM can you please update this PR based on the previous feedback

DZPM · 2018-07-23T12:54:33Z

Sure, will do. (Sorry, I've been busy and the PR slipped my mind...)

DZPM · 2018-08-01T13:41:47Z

Done. Added the changes in a new commit, and rebased.

datapythonista

Looks great, just few minor things. Thanks for the changes!

datapythonista · 2018-08-01T14:11:15Z

pandas/plotting/_core.py

+            ...     'signups': [5, 5, 6, 12, 14, 13],
+            ...     'visits': [20, 42, 28, 62, 81, 50],
+            ... }, index=pd.date_range(start='2018/01/01', end='2018/07/01',
+            ...                        freq='M'))


is the .. plot:: directive resetting the namespace? Otherwise, It'd be good to reuse the DataFrame instead of repeating it in every example.

Had some issues with the namespace, so I kept it in different places.

Don't have the full environment right now, can't check if it also works the other way.

Recreating the dataframe each time should not be needed

datapythonista · 2018-08-01T14:13:37Z

pandas/plotting/_core.py

+
+        See Also
+        --------
+        pandas.DataFrame.plot : Draw plots.


You can get rid of pandas. and just leave DataFrame.plot. Something a bit more descriptive than "Draw plots." could be useful.

Not sure if in similar docstrings we also added the matplotlib equivalent method. Can you check the docstring for DataFrame.kde for example, and be consistent with it?

Added the DataFrame.plot description.

datapythonista · 2018-08-01T14:16:41Z

pandas/plotting/_core.py

-        `**kwds` : optional
+        x : label or position, optional
+            Coordinates for the X axis. By default uses the index.
+        y : label or position, optional


As @dukebody says, for both x and y, in the first line we should have the type, more than a short description. It's a bit tricky, as label can be anything, but I think we're using something like object or int, optional. Then, we can sure clarify that it's the label or the position in the description.

I kept it consistent with DataFrame.plot, which is the more complete docstring example I could find.

datapythonista · 2018-08-01T14:18:42Z

pandas/plotting/_core.py

+            Coordinates for the X axis. By default uses the index.
+        y : label or position, optional
+            Column to plot. By default uses all columns.
+        stacked : boolean, default True


Happy to leave like this now, but as stacked is not in the signature, may be we should add it there, or document it under kwds. But if we leave it like this, can you replace boolean by the Python type bool please?

DZPM · 2018-08-08T16:08:29Z

Changes done, and rebased. Hope it's ok!

TomAugspurger · 2018-08-10T17:07:38Z

LGTM, thanks!

If you want, check the doc build when it finishes (probably a few hours).

jorisvandenbossche

Some follow-up in #22307

jorisvandenbossche · 2018-08-13T09:52:07Z

pandas/plotting/_core.py

+            ...     'signups': [5, 5, 6, 12, 14, 13],
+            ...     'visits': [20, 42, 28, 62, 81, 50],
+            ... }, index=pd.date_range(start='2018/01/01', end='2018/07/01',
+            ...                        freq='M'))


Recreating the dataframe each time should not be needed

jorisvandenbossche · 2018-08-13T09:54:17Z

pandas/plotting/_core.py

+            ...     'visits': [20, 42, 28],
+            ...     'day': ['Monday', 'Tuesday', 'Wednesday'],
+            ... }, index=pd.date_range(start='2018/01/01', end='2018/07/01',
+            ...                        freq='M'))


This example is not running (the index is not correct). And also the labels to not show on the actual plot once this is fixed.

kynan reviewed Mar 10, 2018

View reviewed changes

jorisvandenbossche reviewed Mar 10, 2018

View reviewed changes

dukebody suggested changes Mar 10, 2018

View reviewed changes

jorisvandenbossche added the Docs label Mar 12, 2018

DZPM force-pushed the docstring_dataframe_plot_area branch from 6a4c50c to 9223d2f Compare March 15, 2018 11:22

DZPM force-pushed the docstring_dataframe_plot_area branch from 9223d2f to b59f433 Compare March 15, 2018 12:24

jorisvandenbossche reviewed Mar 15, 2018

View reviewed changes

DZPM force-pushed the docstring_dataframe_plot_area branch from b59f433 to 5077a12 Compare March 26, 2018 20:16

jorisvandenbossche reviewed Apr 9, 2018

View reviewed changes

datapythonista self-assigned this Jul 22, 2018

DZPM force-pushed the docstring_dataframe_plot_area branch from 5077a12 to 86db52e Compare August 1, 2018 13:40

datapythonista reviewed Aug 1, 2018

View reviewed changes

DZPM added 3 commits August 8, 2018 16:50

DOC: Improve docstring for pandas.DataFrame.plot.area

bb46fea

Improved with suggestions

a0df497

More improvements

3f02fe7

DZPM force-pushed the docstring_dataframe_plot_area branch from 86db52e to 3f02fe7 Compare August 8, 2018 16:06

jreback added this to the 0.24.0 milestone Aug 9, 2018

TomAugspurger merged commit c04b773 into pandas-dev:master Aug 10, 2018

jorisvandenbossche mentioned this pull request Aug 13, 2018

DOC: fixes to DataFrame.plot.area docstring #22307

Merged

jorisvandenbossche reviewed Aug 13, 2018

View reviewed changes

Sup3rGeo pushed a commit to Sup3rGeo/pandas that referenced this pull request Oct 1, 2018

DOC: update the pandas.DataFrame.plot.area docstring (pandas-dev#20170)

0466dd3

DOC: update the pandas.DataFrame.plot.area docstring #20170

DOC: update the pandas.DataFrame.plot.area docstring #20170

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

jorisvandenbossche left a 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

dukebody commented Mar 10, 2018

dukebody left a 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

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

dukebody commented Mar 13, 2018

DZPM commented Mar 13, 2018

codecov bot commented Mar 15, 2018 • edited Loading

Codecov Report

DZPM commented Mar 15, 2018

jorisvandenbossche left a 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

TomAugspurger commented Mar 15, 2018

TomAugspurger commented Mar 15, 2018

DZPM commented Mar 26, 2018

DZPM commented Apr 6, 2018

jorisvandenbossche commented Apr 9, 2018

jorisvandenbossche left a 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

datapythonista commented Jul 22, 2018

DZPM commented Jul 23, 2018

DZPM commented Aug 1, 2018

datapythonista left a 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 Aug 8, 2018

TomAugspurger commented Aug 10, 2018

jorisvandenbossche left a 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 15, 2018 •

edited

Loading