Ensure conversion to "native" types for integer EA #31328

rushabh-v · 2020-01-26T16:31:34Z

closes API: ExtensionArrays and conversion to "native" types (eg in tolist, to_dict, iteration, ..) #29738
tests added / passed
passes black pandas

MarcoGorelli · 2020-01-26T18:23:18Z

pandas/tests/arrays/test_integer.py

+    result = type(pd.Series([1, 2], dtype="int64").tolist()[0])
+    assert expected == result


I think it's generally preferred to use isinstance, see https://www.python.org/dev/peps/pep-0008/

Object type comparisons should always use isinstance() instead of comparing types directly.

Yes: if isinstance(obj, int):

No: if type(obj) is type(1):

But that would become isinstance(int, int) in this case, which returns False.
Are you asking to do it some other way?

What I meant was

isinstance(pd.Series([1, 2], dtype="int64").tolist()[0], int)

Oh, okay. I will commit that soon.

I have done that. Can you review it, please?

rushabh-v · 2020-01-26T19:31:31Z

One test is failing. see, https://dev.azure.com/pandas-dev/pandas/_build/results?buildId=26857&view=logs&j=bef1c175-2c1b-51ae-044a-2437c76fc339&t=770e7bb1-09f5-5ebf-b63b-578d2906aac9&l=169

I think it is because the series is being converted into int8 dtype somehow. Any thoughts?

MarcoGorelli · 2020-02-09T10:19:16Z

Hey @rushabh-v - sorry for only getting round to this now.

One test is failing. see, https://dev.azure.com/pandas-dev/pandas/_build/results?buildId=26857&view=logs&j=bef1c175-2c1b-51ae-044a-2437c76fc339&t=770e7bb1-09f5-5ebf-b63b-578d2906aac9&l=169

The build no longer exists. I'll just make another couple of suggestions, if you then ping me when you've done them I'll take a look at the failing tests

MarcoGorelli · 2020-02-09T10:21:23Z

pandas/tests/arrays/test_integer.py

+def test_integer_Series_iter_return_native():
+    assert isinstance(pd.Series([1, 2], dtype="int64").tolist()[0], int)
+    assert isinstance(pd.Series([1, 2], dtype="Int64").tolist()[0], int)
+    assert isinstance(pd.Series([1, 2], dtype="int64").to_dict()[0], int)
+    assert isinstance(pd.Series([1, 2], dtype="Int64").to_dict()[0], int)
+    assert isinstance(list(pd.Series([1, 2], dtype="int64").iteritems())[0][1], int)
+    assert isinstance(list(pd.Series([1, 2], dtype="Int64").iteritems())[0][1], int)
+    assert isinstance(list(iter(pd.Series([1, 2], dtype="int64")))[0], int)
+    assert isinstance(list(iter(pd.Series([1, 2], dtype="Int64")))[0], int)


Please leave a comment with the issue number, so

def test_integer_Series_iter_return_native(): # GH <issue number goes here>

Can this test be parametrised somewhat? See here, as well as several cases in the pandas tests (e.g. the one below this one) for examples. There's some examples in the contributing guide too

MarcoGorelli · 2020-02-09T10:23:42Z

Also, a whatsnew entry will be required (v1.1.0, I believe)

rushabh-v · 2020-02-09T12:31:56Z

@MarcoGorelli I have made all the changes. Can you please take a look at the logs now? see https://dev.azure.com/pandas-dev/pandas/_build/results?buildId=28135&view=logs&j=bef1c175-2c1b-51ae-044a-2437c76fc339&t=770e7bb1-09f5-5ebf-b63b-578d2906aac9&l=125

MarcoGorelli · 2020-02-10T22:08:14Z

Sure, I'll look at this later this week.

(note to self: to reproduce the error:

pytest pandas/tests/extension/test_integer.py

)

MarcoGorelli · 2020-02-11T21:53:42Z

I think there might be another underlying problem here, which I've raised in #31899.

rushabh-v · 2020-02-15T07:36:24Z

So can you review and merge this PR now or we should wait for #31899 to be resolved?

MarcoGorelli · 2020-02-15T08:34:52Z

Yes, the tests should be passing before we can merge. I'll look into 31899 today

MarcoGorelli · 2020-02-15T19:21:53Z

Current tests fail because of this:

>>> import pandas as pd
>>> s1 = pd.Series([100], dtype='Int8')                                                                     
>>> s2 = pd.Series([100], dtype='Int8')

So, without the current fix, we have

>>> [a+b for (a, b) in zip(s1.values, s2.values)]
[-56]
>>> s1.combine(s2, lambda x, y: x+y)                                                                        
0    -56
dtype: Int64

With the current fix:

>>> [a+b for (a, b) in zip(s1.values, s2.values)]
[200]
>>> s1.combine(s2, lambda x, y: x+y)                                                                        
0    -56
dtype: Int64

cc @jorisvandenbossche

WillAyd · 2020-03-14T22:10:57Z

@rushabh-v is this still active? Can you merge master and try to get green?

…-native

rushabh-v · 2020-03-15T12:15:50Z

still fails

rushabh-v · 2020-05-14T17:49:12Z

Any updates?

MarcoGorelli · 2020-05-14T17:58:19Z

Not from me - I removed the 'good first issue' tag from the original issue as I think there's some underlying issues that need to be solved here first, and they aren't so easy

jreback · 2020-06-14T17:20:29Z

@rushabh-v can you merge master

jreback · 2020-06-14T17:21:27Z

pandas/core/arrays/integer.py

@@ -354,6 +354,13 @@ def __init__(self, values: np.ndarray, mask: np.ndarray, copy: bool = False):
            )
        super().__init__(values, mask, copy=copy)

+    def __iter__(self):


I think we could actually move this to base masked and/or the base EA interface (maybe)

jreback · 2020-10-24T03:01:42Z

closing in favor of #37377

rushabh-v added 3 commits January 26, 2020 21:55

add .item() to__iter__ and add tests

a7bda74

rum black pandas

0b01ace

add __iter__ to IntegerArray

735224a

MarcoGorelli reviewed Jan 26, 2020

View reviewed changes

rushabh-v requested a review from jorisvandenbossche January 26, 2020 19:31

use isinstance

9dac007

rushabh-v requested a review from MarcoGorelli February 9, 2020 07:55

MarcoGorelli requested changes Feb 9, 2020

View reviewed changes

rushabh-v added 2 commits February 9, 2020 16:38

parametrize

b098f4f

add whatsnew

a5d64c4

rushabh-v requested a review from MarcoGorelli February 10, 2020 18:05

Merge branch 'master' of https://github.com/pandas-dev/pandas into EA…

151fdc6

…-native

jreback added ExtensionArray Extending pandas with custom dtypes or arrays. Indexing Related to indexing on series/frames, not to indexes themselves labels Jun 14, 2020

jreback reviewed Jun 14, 2020

View reviewed changes

dsaxton added the Stale label Sep 17, 2020

arw2019 mentioned this pull request Oct 24, 2020

ENH/BUG: implement __iter__ for IntegerArray so conversions (to_dict, tolist, etc.) return python native types #37377

Closed

6 tasks

jreback added this to the No action milestone Oct 24, 2020

jreback closed this Oct 24, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ensure conversion to "native" types for integer EA #31328

Ensure conversion to "native" types for integer EA #31328

rushabh-v commented Jan 26, 2020 •

edited

Loading

MarcoGorelli Jan 26, 2020

rushabh-v Jan 26, 2020 •

edited

Loading

MarcoGorelli Jan 26, 2020

rushabh-v Jan 26, 2020

rushabh-v Feb 9, 2020

rushabh-v commented Jan 26, 2020

MarcoGorelli commented Feb 9, 2020 •

edited

Loading

MarcoGorelli Feb 9, 2020

MarcoGorelli commented Feb 9, 2020

rushabh-v commented Feb 9, 2020

MarcoGorelli commented Feb 10, 2020

MarcoGorelli commented Feb 11, 2020

rushabh-v commented Feb 15, 2020

MarcoGorelli commented Feb 15, 2020

MarcoGorelli commented Feb 15, 2020 •

edited

Loading

WillAyd commented Mar 14, 2020

rushabh-v commented Mar 15, 2020

rushabh-v commented May 14, 2020

MarcoGorelli commented May 14, 2020

jreback commented Jun 14, 2020

jreback Jun 14, 2020

jreback commented Oct 24, 2020

		result = type(pd.Series([1, 2], dtype="int64").tolist()[0])
		assert expected == result

Ensure conversion to "native" types for integer EA #31328

Ensure conversion to "native" types for integer EA #31328

Conversation

rushabh-v commented Jan 26, 2020 • edited Loading

MarcoGorelli Jan 26, 2020

Choose a reason for hiding this comment

rushabh-v Jan 26, 2020 • edited Loading

Choose a reason for hiding this comment

MarcoGorelli Jan 26, 2020

Choose a reason for hiding this comment

rushabh-v Jan 26, 2020

Choose a reason for hiding this comment

rushabh-v Feb 9, 2020

Choose a reason for hiding this comment

rushabh-v commented Jan 26, 2020

MarcoGorelli commented Feb 9, 2020 • edited Loading

MarcoGorelli Feb 9, 2020

Choose a reason for hiding this comment

MarcoGorelli commented Feb 9, 2020

rushabh-v commented Feb 9, 2020

MarcoGorelli commented Feb 10, 2020

MarcoGorelli commented Feb 11, 2020

rushabh-v commented Feb 15, 2020

MarcoGorelli commented Feb 15, 2020

MarcoGorelli commented Feb 15, 2020 • edited Loading

WillAyd commented Mar 14, 2020

rushabh-v commented Mar 15, 2020

rushabh-v commented May 14, 2020

MarcoGorelli commented May 14, 2020

jreback commented Jun 14, 2020

jreback Jun 14, 2020

Choose a reason for hiding this comment

jreback commented Oct 24, 2020

rushabh-v commented Jan 26, 2020 •

edited

Loading

rushabh-v Jan 26, 2020 •

edited

Loading

MarcoGorelli commented Feb 9, 2020 •

edited

Loading

MarcoGorelli commented Feb 15, 2020 •

edited

Loading