BUG: Fixes plotting with nullable integers (#32073) #38014

rkc007 · 2020-11-23T06:47:28Z

closes Plotting Int64 columns with nulled integers (NAType) fails #32073
tests added / passed
passes black pandas
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

rkc007 · 2020-11-23T06:48:37Z

This is the first time I am contributing to pandas open source project. Please let me know if I can improve anything on this commit.

arw2019 · 2020-11-23T19:54:30Z

doc/source/whatsnew/v1.2.0.rst

 indexed by a :class:`TimedeltaIndex` with a fixed frequency when x-axis lower limit was greater than upper limit (:issue:`37454`)
 - Bug in :meth:`DataFrameGroupBy.boxplot` when ``subplots=False``, a KeyError would raise (:issue:`16748`)
 - Bug in :meth:`DataFrame.plot` and :meth:`Series.plot` was overwriting matplotlib's shared y axes behaviour when no sharey parameter was passed (:issue:`37942`)
+- Bug in :meth:`DataFrame.plot` to handle nullable integers (:issue:`32073`)


We want to be more specific:

Bug in :meth:`DataFrame.plot` was raising a ``TypeError`` with ``ExtensionDtype`` columns (:issue:`32073`)

Thank you @arw2019 for your comments. I will add these changes to the next commit.

@arw2019 I added the changes to the latest commit. Can you please review it again ?

arw2019

Thanks @rkc007 for the PR!

Some comments. Also take a look at the CI (we standardize the use of pd.DataFrame/DataFrame within a file so having both errors)

Let us know if you have any questions!

arw2019 · 2020-11-23T19:54:44Z

pandas/plotting/_matplotlib/core.py

- numeric_data = numeric_data.copy()
- for col in numeric_data:
- numeric_data[col] = np.asarray(numeric_data[col])
+ def convert_to_ndarray(data):


you could use pd.to_numeric here I think. Illustrative example:

In [8]: s = pd.Series([1, 2, 3, pd.NA], dtype="Int64") In [9]: pd.to_numeric(s.to_numpy(), downcast="integer") Out[9]: array([ 1., 2., 3., nan])

Thank you @arw2019 for the example. I think I didn't get your point here. Can you please explain it a bit more?

no don't use pd.to_numeric we already know the type and its an internal function

arw2019 · 2020-11-23T19:56:32Z

pandas/tests/plotting/frame/test_frame.py

+ )
+
+ _check_plot_works(df.plot, x="A", y="B")
+ _check_plot_works(df[["A", "B"]].astype("Int64").plot, x="A", y="B")


I think we'd rather to specify the dtype in the dataframe construction

Sure. Thank you for the suggestion. I will rectify this in the next commit.

@arw2019 I added the changes to the latest commit. Can you please review it again ?

ivanovmg

@rkc007, please see my comments below.

ivanovmg · 2020-11-24T19:54:11Z

pandas/plotting/_matplotlib/core.py

- numeric_data = numeric_data.copy()
- for col in numeric_data:
- numeric_data[col] = np.asarray(numeric_data[col])
+ def convert_to_ndarray(data):


Consider moving convert_to_ndarray out of the scope of the function _compute_plot_data.

yes pls do this

@jreback I added the changes to the latest commit. Can you please review it again ?

ivanovmg · 2020-11-24T19:55:55Z

pandas/tests/plotting/frame/test_frame.py

+ "A": [1, 2, 3, 4, 5],
+ "B": [7, 5, np.nan, 3, 2],


What about having a nullable value in column "A" (x-axis)?

@ivanovmg I added the changes to the latest commit. Can you please review it again ?

jreback · 2020-11-26T16:57:33Z

pandas/plotting/_matplotlib/core.py

- numeric_data = numeric_data.copy()
- for col in numeric_data:
- numeric_data[col] = np.asarray(numeric_data[col])
+ def convert_to_ndarray(data):


no don't use pd.to_numeric we already know the type and its an internal function

jreback · 2020-11-26T16:57:38Z

pandas/plotting/_matplotlib/core.py

- numeric_data = numeric_data.copy()
- for col in numeric_data:
- numeric_data[col] = np.asarray(numeric_data[col])
+ def convert_to_ndarray(data):


yes pls do this

jreback · 2020-11-26T16:58:19Z

pandas/plotting/_matplotlib/core.py

- numeric_data[col] = np.asarray(numeric_data[col])
+ def convert_to_ndarray(data):
+ # GH32073: cast to float if values contain nulled integers
+ if is_integer_dtype(data.dtype) and is_extension_array_dtype(data.dtype):


actually we want to also catch / test nullable floats as well

@jreback I fixed this in the latest commit.

ivanovmg · 2020-12-03T03:18:32Z

pandas/plotting/_matplotlib/core.py

 return self.axes[0]

+ def _convert_to_ndarray(self, data):
+ # data = self.data


Please delete this comment.

Thanks for pointing this out @ivanovmg.
Deleted the comment in the latest commit.

ivanovmg

One nitpick about the unnecessary comment.
Otherwise looks good to me.

jreback

looks good. can you merge master and ping on green.

rkc007 · 2020-12-07T00:43:07Z

@jreback the branch has the latest code and all the checks passed.

jreback · 2020-12-07T13:38:34Z

thanks @rkc007

Added checking for NA values

edf7196

arw2019 reviewed Nov 23, 2020

View reviewed changes

arw2019 suggested changes Nov 23, 2020

View reviewed changes

ivanovmg suggested changes Nov 24, 2020

View reviewed changes

jreback added NA - MaskedArrays Related to pd.NA and nullable extension arrays Visualization plotting labels Nov 26, 2020

jreback requested changes Nov 26, 2020

View reviewed changes

rkc007 added 2 commits November 27, 2020 09:05

merger upstream/master to GH32073

5fd6d3e

Added more test cases & changed plotting/core method

22e6df8

rkc007 requested review from arw2019, ivanovmg and jreback December 2, 2020 21:02

ivanovmg reviewed Dec 3, 2020

View reviewed changes

ivanovmg approved these changes Dec 3, 2020

View reviewed changes

rkc007 added 2 commits December 2, 2020 23:45

Merge remote-tracking branch 'upstream/master' into GH32073

89cde5d

removed unnecessary comment

6d70524

jreback added this to the 1.2 milestone Dec 4, 2020

jreback approved these changes Dec 4, 2020

View reviewed changes

Merge remote-tracking branch 'upstream/master' into GH32073

0db421f

jreback merged commit 0aa598a into pandas-dev:master Dec 7, 2020

rkc007 deleted the GH32073 branch December 8, 2020 02:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

BUG: Fixes plotting with nullable integers (#32073) #38014

BUG: Fixes plotting with nullable integers (#32073) #38014

Uh oh!

rkc007 commented Nov 23, 2020

rkc007 commented Nov 23, 2020

arw2019 Nov 23, 2020

rkc007 Nov 24, 2020

rkc007 Nov 27, 2020

arw2019 left a comment

arw2019 Nov 23, 2020

rkc007 Nov 24, 2020

jreback Nov 26, 2020

arw2019 Nov 23, 2020

rkc007 Nov 24, 2020

rkc007 Nov 27, 2020

ivanovmg left a comment

ivanovmg Nov 24, 2020

jreback Nov 26, 2020

rkc007 Nov 27, 2020

ivanovmg Nov 24, 2020

rkc007 Nov 27, 2020

jreback Nov 26, 2020

jreback Nov 26, 2020

jreback Nov 26, 2020

rkc007 Nov 27, 2020

ivanovmg Dec 3, 2020

rkc007 Dec 3, 2020

ivanovmg left a comment

jreback left a comment

rkc007 commented Dec 7, 2020

jreback commented Dec 7, 2020

Labels

4 participants

Uh oh!

BUG: Fixes plotting with nullable integers (#32073) #38014

BUG: Fixes plotting with nullable integers (#32073) #38014

Uh oh!

Conversation

rkc007 commented Nov 23, 2020

rkc007 commented Nov 23, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arw2019 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

ivanovmg 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

ivanovmg left a comment

Choose a reason for hiding this comment

jreback left a comment

Choose a reason for hiding this comment

rkc007 commented Dec 7, 2020

jreback commented Dec 7, 2020

Labels

4 participants