DOC: Exposed arguments in plot.kde #19229

tommyod · 2018-01-13T18:47:14Z

The documentation for plot.kde did not show the bw_method and ind arguments, which are specific to plot.kde (and plot.density, which refers to the same method).

There is also a change in the actual code, and a corresponding test added. I added an option for ind to be an integer number of sample points, instead of necessarily a np.array. I image that a user typically just wants a number of equidistant sample points, and does not want to construct an array.

tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

Comments very welcome.

pep8speaks · 2018-01-13T18:47:16Z

Hello @tommyod! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on February 01, 2018 at 14:12 Hours UTC

tommyod · 2018-01-13T18:50:07Z

Ran the flake8 tool while in the wrong subdirectory locally. Will fix the errors tomorrow and push.

TomAugspurger

Thanks. Could fixup the linting errors, and add a release note in whatsnew/v0.23.0.txt?

TomAugspurger · 2018-01-13T20:34:16Z

pandas/plotting/_core.py

@@ -1398,6 +1398,10 @@ def _get_ind(self, y):
            sample_range = np.nanmax(y) - np.nanmin(y)
            ind = np.linspace(np.nanmin(y) - 0.5 * sample_range,
                              np.nanmax(y) + 0.5 * sample_range, 1000)
+        elif isinstance(self.ind, (int, np.int)):


Use isinstance(self.ind, numbers.Integral). It should catch both of these (may have to import numbers).

instead of this, just use is_integer, which is our version of this and handles all the cases.
import from pandas.core.dtypes.common (if not already imported)

TomAugspurger · 2018-01-13T20:35:19Z

pandas/tests/plotting/test_series.py

@@ -622,6 +622,7 @@ def test_kde_kwargs(self):
            pytest.skip("mpl is not supported")

        from numpy import linspace
+        _check_plot_works(self.ts.plot.kde, bw_method=None, ind=20)


Could you add a test with np.int(20) as well.

can you add a use with bw_method as a string.

codecov · 2018-01-14T06:56:39Z

Codecov Report

Merging #19229 into master will decrease coverage by <.01%.
The diff coverage is 42.85%.

@@            Coverage Diff             @@
##           master   #19229      +/-   ##
==========================================
- Coverage   91.67%   91.67%   -0.01%     
==========================================
  Files         148      148              
  Lines       48553    48556       +3     
==========================================
  Hits        44513    44513              
- Misses       4040     4043       +3

Flag	Coverage Δ
#multiple	`90.03% <42.85%> (-0.01%)`	⬇️
#single	`41.71% <28.57%> (-0.01%)`	⬇️

Impacted Files	Coverage Δ
pandas/plotting/_core.py	`82.23% <42.85%> (-0.18%)`	⬇️

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 35812ea...826a124. Read the comment docs.

tommyod · 2018-01-14T08:39:01Z

@jreback, @TomAugspurger - I have addressed the comments.

Wrote an additional test.
Fixed linting errors.
Type checked with is_integer.
Added whatsnew comment.

tommyod · 2018-01-20T12:46:19Z

@jreback Friendly reminder to consider this PR, in case it has been forgotten. If there is anything else that needs to be done, please let me know.

TomAugspurger · 2018-01-20T12:50:26Z

pandas/plotting/_core.py

+            The method used to calculate the estimator bandwidth.  This can be
+            'scott', 'silverman', a scalar constant or a callable.
+            If None (default), 'scott' is used.
+            See :scipy:class:`stats.gaussian_kde` for more information.


Have you verified that this link works? I thought it would be

:func:`scipy.stats.gaussian_kde`.

Agreed. Changed it.

jreback

small changes.

jreback · 2018-01-21T16:32:07Z

doc/source/whatsnew/v0.23.0.txt

@@ -478,7 +478,7 @@ Plotting

 - :func: `DataFrame.plot` now raises a ``ValueError`` when the ``x`` or ``y`` argument is improperly formed (:issue:`18671`)
 - Bug in formatting tick labels with ``datetime.time()`` and fractional seconds (:issue:`18478`).
-
+- The arguments ``ind`` and ``bw_method`` are added to the docstring of :meth:`Series.plot.kde` (:issue:`18461`). The argument ``ind`` may now also be an integer (number of sample points).


:meth:`Series.plot.kde` has exposed the args......in the doc-string.

jreback · 2018-01-21T16:32:44Z

pandas/tests/plotting/test_series.py

@@ -622,6 +622,7 @@ def test_kde_kwargs(self):
            pytest.skip("mpl is not supported")

        from numpy import linspace
+        _check_plot_works(self.ts.plot.kde, bw_method=None, ind=20)


can you add a use with bw_method as a string.

jreback · 2018-01-21T16:33:01Z

pandas/tests/plotting/test_series.py

@@ -622,6 +622,8 @@ def test_kde_kwargs(self):
            pytest.skip("mpl is not supported")

        from numpy import linspace


don't import here, rather use np.linspace

jreback · 2018-01-21T16:33:29Z

pandas/tests/plotting/test_series.py

@@ -622,6 +622,8 @@ def test_kde_kwargs(self):
            pytest.skip("mpl is not supported")

        from numpy import linspace
+        _check_plot_works(self.ts.plot.kde, bw_method=None, ind=20)
+        _check_plot_works(self.ts.plot.kde, bw_method=None, ind=np.int(20))


ideally would parametrize this test instead of repeating.

jreback · 2018-01-21T19:51:05Z

pandas/plotting/_core.py

        `**kwds` : optional
-            Keyword arguments to pass on to :py:meth:`pandas.DataFrame.plot`.
+            Keyword arguments to pass on to :py:meth:`pandas.Series.plot`.


is this change right? This the DataFrame one

Good catch. My bad. It should be pandas.DataFrame.plot, and I have made the change.

tommyod · 2018-02-02T05:27:46Z

@jreback I believe this PR is good to go now.

TomAugspurger · 2018-02-02T12:50:54Z

Thanks @tommyod!

* Exposed arguments in plot.kde, added number of sample points as option * Added a test for plot.kde with as an integer * Added whatsnew. Fixed flake8 errors. Used is_integer to infer type. * Updated scipy reference * Added test, rewrote whatsnew, removed import * Changed from Series to DataFrame in doc * Fixed PEP8 errors in test file * Fixed typo which made tests crash

TomAugspurger reviewed Jan 13, 2018

View reviewed changes

jreback added Docs Visualization plotting labels Jan 13, 2018

TomAugspurger reviewed Jan 20, 2018

View reviewed changes

jreback requested changes Jan 21, 2018

View reviewed changes

jreback added this to the 0.23.0 milestone Jan 21, 2018

jreback requested changes Jan 21, 2018

View reviewed changes

tommyod added 8 commits February 1, 2018 15:11

Exposed arguments in plot.kde, added number of sample points as option

bcea8ff

Added a test for plot.kde with as an integer

1961718

Added whatsnew. Fixed flake8 errors. Used is_integer to infer type.

97e7b83

Updated scipy reference

1d14492

Added test, rewrote whatsnew, removed import

0e9cb37

Changed from Series to DataFrame in doc

b2d3ab1

Fixed PEP8 errors in test file

a56b19b

Fixed typo which made tests crash

826a124

TomAugspurger merged commit 69cd5fb into pandas-dev:master Feb 2, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DOC: Exposed arguments in plot.kde #19229

DOC: Exposed arguments in plot.kde #19229

tommyod commented Jan 13, 2018 •

edited

Loading

pep8speaks commented Jan 13, 2018 •

edited

Loading

tommyod commented Jan 13, 2018

TomAugspurger left a comment

TomAugspurger Jan 13, 2018

jreback Jan 13, 2018

TomAugspurger Jan 13, 2018

jreback Jan 21, 2018

codecov bot commented Jan 14, 2018 •

edited

Loading

tommyod commented Jan 14, 2018

tommyod commented Jan 20, 2018

TomAugspurger Jan 20, 2018

tommyod Jan 21, 2018

jreback left a comment

jreback Jan 21, 2018

jreback Jan 21, 2018

jreback Jan 21, 2018

jreback Jan 21, 2018

jreback Jan 21, 2018

tommyod Jan 21, 2018

tommyod commented Feb 2, 2018

TomAugspurger commented Feb 2, 2018

		@@ -622,6 +622,8 @@ def test_kde_kwargs(self):
		pytest.skip("mpl is not supported")

		from numpy import linspace

DOC: Exposed arguments in plot.kde #19229

DOC: Exposed arguments in plot.kde #19229

Conversation

tommyod commented Jan 13, 2018 • edited Loading

pep8speaks commented Jan 13, 2018 • edited Loading

Comment last updated on February 01, 2018 at 14:12 Hours UTC

tommyod commented Jan 13, 2018

TomAugspurger 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

codecov bot commented Jan 14, 2018 • edited Loading

Codecov Report

tommyod commented Jan 14, 2018

tommyod commented Jan 20, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback 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

tommyod commented Feb 2, 2018

TomAugspurger commented Feb 2, 2018

tommyod commented Jan 13, 2018 •

edited

Loading

pep8speaks commented Jan 13, 2018 •

edited

Loading

codecov bot commented Jan 14, 2018 •

edited

Loading