TST: Inconsistent behavior of .replace() in Int64 series with NA #38693

ftrihardjo · 2020-12-25T08:47:35Z

closes BUG: Inconsistent behavior of .replace() in Int64 series with <NA>. #38267
tests added / passed
passes black pandas
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

MarcoGorelli

MarcoGorelli · 2020-12-25T18:06:17Z

pandas/tests/series/methods/test_replace.py

+    def test_replace_int_with_na(self, dtype):
+        result = pd.Series([0, None]).astype(dtype)


can you put the issue number as a comment here? see some other tests for an example

MarcoGorelli · 2020-12-25T18:07:35Z

pandas/tests/series/methods/test_replace.py

+    @pytest.mark.parametrize('dtype', ['int8', 'int16', 'int32', 'int64'])
+    def test_replace_int_with_na(self, dtype):
+        result = pd.Series([0, None]).astype(dtype)
+        result.replace(0, pd.NA)


I think you'll want

result = result.replace(0, pd.NA)

MarcoGorelli · 2020-12-25T18:07:53Z

pandas/tests/series/methods/test_replace.py

+        result = pd.Series([0, None]).astype(dtype)
+        result.replace(0, pd.NA)
+        expected = pd.Series([0, None]).astype(dtype)
+        expected.fillna(0).replace(0, pd.NA)


why is this line needed? Just construct expected directly

ftrihardjo · 2020-12-26T02:09:37Z

thank you for your review <https://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail> Virus-free. www.avast.com <https://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail> <#DAB4FAD8-2DD7-40BB-A1B8-4E2AA1F9FDF2>

…

On Sat, Dec 26, 2020 at 1:09 AM Marco Gorelli ***@***.***> wrote: ***@***.**** requested changes on this pull request. Thanks @ftrihardjo <https://github.com/ftrihardjo> ------------------------------ In pandas/tests/series/methods/test_replace.py <#38693 (comment)>: > + def test_replace_int_with_na(self, dtype): + result = pd.Series([0, None]).astype(dtype) can you put the issue number as a comment here? see some other tests for an example ------------------------------ In pandas/tests/series/methods/test_replace.py <#38693 (comment)>: > @@ -208,6 +208,14 @@ def test_replace_with_dict_with_bool_keys(self): expected = pd.Series(["yes", False, "yes"]) tm.assert_series_equal(result, expected) + @pytest.mark.parametrize('dtype', ['int8', 'int16', 'int32', 'int64']) + def test_replace_int_with_na(self, dtype): + result = pd.Series([0, None]).astype(dtype) + result.replace(0, pd.NA) I think you'll want result = result.replace(0, pd.NA) ------------------------------ In pandas/tests/series/methods/test_replace.py <#38693 (comment)>: > @@ -208,6 +208,14 @@ def test_replace_with_dict_with_bool_keys(self): expected = pd.Series(["yes", False, "yes"]) tm.assert_series_equal(result, expected) + @pytest.mark.parametrize('dtype', ['int8', 'int16', 'int32', 'int64']) + def test_replace_int_with_na(self, dtype): + result = pd.Series([0, None]).astype(dtype) + result.replace(0, pd.NA) + expected = pd.Series([0, None]).astype(dtype) + expected.fillna(0).replace(0, pd.NA) why is this line needed? Just construct expected directly — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#38693 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AF2RQQOYKEZAMDBFVPBC7UDSWTILXANCNFSM4VI6XQWA> .

MarcoGorelli · 2020-12-26T08:38:26Z

pandas/tests/series/methods/test_replace.py

+    def test_replace_int_with_na(self, dtype):
+        # GH 38267
+        result = pd.Series([0, None]).astype(dtype).replace(0, pd.NA)
+        expected = pd.Series([0, None]).astype(dtype).fillna(0).replace(0, pd.NA)


Is it possible to construct expected directly (e.g. expected = pd.Series([pd.NA, pd.NA]))?

MarcoGorelli · 2020-12-26T13:16:35Z

pandas/tests/series/methods/test_replace.py

@@ -208,6 +208,13 @@ def test_replace_with_dict_with_bool_keys(self):
        expected = pd.Series(["yes", False, "yes"])
        tm.assert_series_equal(result, expected)

+    @pytest.mark.parametrize("dtype", ["int8", "int16", "int32", "int64"])


the types need to be capitalised, as in the issue, e.g. Int64 not int64

arw2019 · 2020-12-27T06:58:56Z

pandas/tests/series/methods/test_replace.py

@@ -208,6 +208,13 @@ def test_replace_with_dict_with_bool_keys(self):
        expected = pd.Series(["yes", False, "yes"])
        tm.assert_series_equal(result, expected)

+    @pytest.mark.parametrize("dtype", ["Int8", "Int16", "Int32", "Int64"])
+    def test_replace_int_with_na(self, dtype):


should use any_nullable_int_dtype fixture here

arw2019 · 2020-12-27T06:59:55Z

pandas/tests/series/methods/test_replace.py

+    @pytest.mark.parametrize("dtype", ["Int8", "Int16", "Int32", "Int64"])
+    def test_replace_int_with_na(self, dtype):
+        # GH 38267
+        result = pd.Series([0, None]).astype(dtype).replace(0, pd.NA)


pass dtype directly to the Series constructor

arw2019 · 2020-12-27T07:00:31Z

pandas/tests/series/methods/test_replace.py

@@ -208,6 +208,13 @@ def test_replace_with_dict_with_bool_keys(self):
        expected = pd.Series(["yes", False, "yes"])
        tm.assert_series_equal(result, expected)

+    @pytest.mark.parametrize("dtype", ["Int8", "Int16", "Int32", "Int64"])
+    def test_replace_int_with_na(self, dtype):


I'd call it test_replace_nullable_int_with_na or test_replace_Int_with_na

arw2019 · 2020-12-27T07:03:47Z

pandas/tests/series/methods/test_replace.py

+    def test_replace_int_with_na(self, dtype):
+        # GH 38267
+        result = pd.Series([0, None]).astype(dtype).replace(0, pd.NA)
+        expected = pd.Series([pd.NA, pd.NA])


you also need to pass dtype here else (looking at CI output) this casts to object. That's why currently this test doesn't actually pass

MarcoGorelli · 2020-12-27T12:52:32Z

Thanks @ftrihardjo , looks like just #38693 (comment) still needs to be addressed

jreback · 2020-12-27T22:24:21Z

pandas/tests/series/methods/test_replace.py

+    @pytest.mark.parametrize("dtype", ["Int8", "Int16", "Int32", "Int64"])
+    def test_replace_Int_with_na(self, dtype):
+        # GH 38267
+        result = pd.Series([0, None], dtype=dtype).replace(0, pd.NA)


can you add the working example as well e.g. 0,1 as another case.

jreback · 2020-12-27T22:25:31Z

pandas/tests/series/methods/test_replace.py

@@ -208,6 +208,13 @@ def test_replace_with_dict_with_bool_keys(self):
        expected = pd.Series(["yes", False, "yes"])
        tm.assert_series_equal(result, expected)

+    @pytest.mark.parametrize("dtype", ["Int8", "Int16", "Int32", "Int64"])


can you use the fixture: any_nullable_int_dtype instead here.

pep8speaks · 2020-12-27T22:31:26Z

Hello @ftrihardjo! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-12-28 02:00:00 UTC

jreback · 2020-12-28T14:55:49Z

thanks @ftrihardjo

…das-dev#38693)

ftrihardjo added 2 commits December 25, 2020 15:42

pandas-dev issue #38267

39ec797

pandas-dev issue #38267

a83dbcb

MarcoGorelli self-requested a review December 25, 2020 11:03

MarcoGorelli suggested changes Dec 25, 2020

View reviewed changes

jreback changed the title ~~pandas-dev issue #38267~~ BUG: Inconsistent behavior of .replace() in Int64 series with <NA>. Dec 25, 2020

ftrihardjo added 3 commits December 26, 2020 09:19

pandas-dev issue #38267

3db04e7

pandas-dev issue #38267

b8ccce5

pandas-dev issue #38267

9b3f075

MarcoGorelli suggested changes Dec 26, 2020

View reviewed changes

pandas-dev issue #38267

9d437ec

MarcoGorelli self-requested a review December 26, 2020 09:10

MarcoGorelli suggested changes Dec 26, 2020

View reviewed changes

pandas-dev issue #38267

76e2fa6

arw2019 reviewed Dec 27, 2020

View reviewed changes

pandas-dev issue #38267

6320412

jreback added Missing-data np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate NA - MaskedArrays Related to pd.NA and nullable extension arrays Testing pandas testing functions or related to the test suite labels Dec 27, 2020

jreback added this to the 1.3 milestone Dec 27, 2020

jreback requested changes Dec 27, 2020

View reviewed changes

pandas-dev issue #38267

a5dc3c4

ftrihardjo added 4 commits December 28, 2020 06:34

pandas-dev issue #38267

ced83a6

pandas-dev issue #38267

d486750

pandas-dev issue #38267

d11fddc

pandas-dev issue #38267

778392c

jorisvandenbossche changed the title ~~BUG: Inconsistent behavior of .replace() in Int64 series with <NA>.~~ TST: Inconsistent behavior of .replace() in Int64 series with NA Dec 28, 2020

MarcoGorelli self-requested a review December 28, 2020 10:28

jreback approved these changes Dec 28, 2020

View reviewed changes

jreback merged commit 0b7ce00 into pandas-dev:master Dec 28, 2020

luckyvs1 pushed a commit to luckyvs1/pandas that referenced this pull request Jan 20, 2021

TST: Inconsistent behavior of .replace() in Int64 series with NA (pan…

bf69b37

…das-dev#38693)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TST: Inconsistent behavior of .replace() in Int64 series with NA #38693

TST: Inconsistent behavior of .replace() in Int64 series with NA #38693

ftrihardjo commented Dec 25, 2020 •

edited by MarcoGorelli

Loading

MarcoGorelli left a comment

MarcoGorelli Dec 25, 2020

MarcoGorelli Dec 25, 2020

MarcoGorelli Dec 25, 2020

ftrihardjo commented Dec 26, 2020 via email

MarcoGorelli Dec 26, 2020

MarcoGorelli Dec 26, 2020

arw2019 Dec 27, 2020

arw2019 Dec 27, 2020

arw2019 Dec 27, 2020

arw2019 Dec 27, 2020

MarcoGorelli commented Dec 27, 2020

jreback Dec 27, 2020

jreback Dec 27, 2020

pep8speaks commented Dec 27, 2020 •

edited

Loading

jreback commented Dec 28, 2020

		def test_replace_int_with_na(self, dtype):
		result = pd.Series([0, None]).astype(dtype)

TST: Inconsistent behavior of .replace() in Int64 series with NA #38693

TST: Inconsistent behavior of .replace() in Int64 series with NA #38693

Conversation

ftrihardjo commented Dec 25, 2020 • edited by MarcoGorelli Loading

MarcoGorelli 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

ftrihardjo commented Dec 26, 2020 via email

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

MarcoGorelli commented Dec 27, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pep8speaks commented Dec 27, 2020 • edited Loading

Comment last updated at 2020-12-28 02:00:00 UTC

jreback commented Dec 28, 2020

ftrihardjo commented Dec 25, 2020 •

edited by MarcoGorelli

Loading

pep8speaks commented Dec 27, 2020 •

edited

Loading