Handling conversion of empty categorical with dtype_backend='pyarrow' #59935

veljanin · 2024-10-02T08:33:57Z

Handling the case where conversion of empty categorical series with specified dtype_backend='pyarrow' in convert_dtypes function results in 1) an error (when there are no defined categories) or 2) in null[pyarrow] dtype (when there are some defined categories).

Tests added and passed
Closes BUG: Trying to use convert_dtypes with dtype_backend='pyarrow' specified on empty categorical series results in error or null[pyarrow] dtype #59934

…pe_backend results in error. Since conversion of non-empty categorical returns categorical of 'numpy_nullable' dtype_backend, now, instead of raising an error, we ensure empty categorical is returned as well.

rhshadrach

Thanks for the PR! Will also need a note in doc/source/whatsnew/v3.0.0 in the Categorical subsection of Bug fixes.

rhshadrach · 2024-10-02T21:01:02Z

pandas/core/dtypes/cast.py

+                if isna(input_array).all() and hasattr(input_array, 'categories'):
+                    inferred_dtype = input_array.dtype
+                else:
+                    inferred_dtype = ArrowDtype(pa_type)


Instead of this, can you add the check not isinstance(input_array.dtype, CategoricalDtype) to the if statement that starts on L1142. Then in the case to categorical dtypes, to_pyarrow_type returns None on L1151 and we do not modify inferred_dtype.

good point!

pandas/tests/frame/methods/test_convert_dtypes.py

rhshadrach · 2024-10-02T21:02:32Z

pandas/tests/frame/methods/test_convert_dtypes.py

+        df = pd.DataFrame(
+            {
+                "A": pd.Series(pd.Categorical([None] * 5)),
+                "B": pd.Series(pd.Categorical([None] * 5, categories=["B1", "B2"])),


No need to wrap in pd.Series here; supplying pd.Cateogrical is sufficient in the constructor.

rhshadrach · 2024-10-02T21:02:49Z

pandas/tests/series/methods/test_convert_dtypes.py

@@ -297,3 +297,16 @@ def test_convert_dtypes_pyarrow_null(self):
        result = ser.convert_dtypes(dtype_backend="pyarrow")
        expected = pd.Series([None, None], dtype=pd.ArrowDtype(pa.null()))
        tm.assert_series_equal(result, expected)
+
+    def test_convert_empty_categorical_to_pyarrow(self):
+        ser = pd.Series(pd.Series(pd.Categorical([None] * 5)))


Similar - You can remove one pd.Series here and can you add # GH#59934 as the first line of this test.

rhshadrach · 2024-10-02T21:04:05Z

pandas/tests/series/methods/test_convert_dtypes.py

+        ser2 = pd.Series(pd.Series(pd.Categorical([None] * 5, categories=["S1", "S2"])))
+        assert (ser2.cat.categories == ["S1", "S2"]).all(), "Series categories are not empty"


What is this testing?

here we need to make sure that the set categories in empty categorical are propagated after conversion... i've adapted this in new pr

veljanin · 2024-10-03T10:59:26Z

Here's the new PR:
#59952

rhshadrach · 2024-10-03T19:33:47Z

@veljanin - please do not open a new PR, just push commits to this PR branch.

veljanin · 2024-10-04T09:54:30Z

All changes should be in place now...

yuanx749 · 2024-10-05T09:38:54Z

doc/source/whatsnew/v3.0.0.rst

@@ -544,7 +544,7 @@ Bug fixes

 Categorical
 ^^^^^^^^^^^
-
+- Bug in :func:`convert_dtypes` with ``dtype_backend='pyarrow'`` parameter where empty categorical series raise error or get converted to null[pyarrow] (:issue:`59934`)


Suggested change

- Bug in :func:`convert_dtypes` with ``dtype_backend='pyarrow'`` parameter where empty categorical series raise error or get converted to null[pyarrow] (:issue:`59934`)

- Bug in :meth:`Series.convert_dtypes` with ``dtype_backend='pyarrow'`` parameter where empty categorical series raise error or get converted to null[pyarrow] (:issue:`59934`)

rhshadrach · 2024-10-05T11:24:40Z

pandas/core/dtypes/cast.py

@@ -1151,6 +1152,7 @@ def convert_dtypes(
                pa_type = to_pyarrow_type(base_dtype)
            if pa_type is not None:
                inferred_dtype = ArrowDtype(pa_type)
+


Can you revert the addition of this newline

…ocs in whatsnew

rhshadrach · 2024-10-07T20:32:51Z

@veljanin - I think changes look good, can you merge main and resolve the conflict.

yuanx749

I think skipping is needed to pass the tests.

pandas/tests/frame/methods/test_convert_dtypes.py

yuanx749 · 2024-10-09T13:51:19Z

pandas/tests/series/methods/test_convert_dtypes.py

@@ -297,3 +297,21 @@ def test_convert_dtypes_pyarrow_null(self):
        result = ser.convert_dtypes(dtype_backend="pyarrow")
        expected = pd.Series([None, None], dtype=pd.ArrowDtype(pa.null()))
        tm.assert_series_equal(result, expected)
+
+    def test_convert_empty_categorical_to_pyarrow(self):


Suggested change

def test_convert_empty_categorical_to_pyarrow(self):

def test_convert_empty_categorical_to_pyarrow(self):

pytest.importorskip("pyarrow")

veljanin · 2024-10-10T15:29:52Z

just added skip_if_no decorator to both tests

…ty_categorical' into fixing_pyarrow_conversion_of_empty_categorical

rhshadrach · 2024-10-12T14:21:25Z

@veljanin - the CI failures are related and need to be addressed.

…ersion_of_empty_categorical

rhshadrach · 2024-10-21T19:47:44Z

ci/code_checks.sh

@@ -73,9 +73,30 @@ if [[ -z "$CHECK" || "$CHECK" == "docstrings" ]]; then
        -i "pandas.Period.freq GL08" \
        -i "pandas.Period.ordinal GL08" \
        -i "pandas.RangeIndex.from_range PR01,SA01" \
+        -i "pandas.Series.cat.add_categories PR01,PR02" \


It looks like merge conflict-resolution went the wrong way here. I think these should likely all be removed.

applied the changes, should be OK now

rhshadrach · 2024-11-07T21:51:44Z

pandas/tests/frame/methods/test_convert_dtypes.py

+        assert converted.A.dtype == "category", "Dtype in column A is not 'category'"
+        assert converted.B.dtype == "category", "Dtype in column B is not 'category'"
+        assert converted.A.cat.categories.empty, "Categories in column A are not empty"
+        assert converted.B.cat.categories.isin(
+            ["B1", "B2"]
+        ).all(), "Categories in column B doesn't contain adequate categories"


These are all covered in tm.assert_frame_equal above and can be removed.

rhshadrach · 2024-11-07T21:52:01Z

pandas/tests/series/methods/test_convert_dtypes.py

+        assert converted1.dtype == "category", "Series dtype is not 'category'"
+        assert converted1.cat.categories.empty, "Series categories are not empty"


Similar, can remove.

rhshadrach · 2024-11-07T21:52:11Z

pandas/tests/series/methods/test_convert_dtypes.py

+        assert converted2.cat.categories.isin(
+            ["S1", "S2"]
+        ).all(), "Categories in ser2 doesn't contain adequate categories"


Similar, can remove.

rhshadrach

This should also get a note in the whatsnew for v3.0.0

github-actions · 2024-12-08T00:08:34Z

This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this.

rhshadrach · 2024-12-08T13:47:16Z

Closing as stale. @veljanin - if you'd like to continue, address the above comments and we're happy to reopen!

Handling the case where converting empty categorical to 'pyarrow' dty…

e6ef750

…pe_backend results in error. Since conversion of non-empty categorical returns categorical of 'numpy_nullable' dtype_backend, now, instead of raising an error, we ensure empty categorical is returned as well.

rhshadrach requested changes Oct 2, 2024

View reviewed changes

rhshadrach added Bug Categorical Categorical Data Type labels Oct 2, 2024

Merge branch 'main' into fixing_pyarrow_conversion_of_empty_categorical

c005850

veljanin mentioned this pull request Oct 3, 2024

Handling conversion of empty categorical with dtype_backend='pyarrow' #59952

Closed

3 tasks

veljanin and others added 3 commits October 4, 2024 10:23

Merge branch 'main' into fixing_pyarrow_conversion_of_empty_categorical

5ce426d

additional revisions

94769a1

removing the change for input_array...

8e879da

yuanx749 suggested changes Oct 5, 2024

View reviewed changes

rhshadrach requested changes Oct 5, 2024

View reviewed changes

reverting newline in Series.convert_dtypes and precising respective d…

a5b3882

…ocs in whatsnew

resolved conflicts and updated main with latest changes

c1ed1f0

veljanin requested review from attack68, MarcoGorelli, WillAyd and mroeschke as code owners October 9, 2024 10:14

Merge branch 'main' into fixing_pyarrow_conversion_of_empty_categorical

6747736

yuanx749 suggested changes Oct 9, 2024

View reviewed changes

adding tm.skip_if_no decorator to tests per feedback

82cecd1

Merge remote-tracking branch 'origin/fixing_pyarrow_conversion_of_emp…

ae0a148

…ty_categorical' into fixing_pyarrow_conversion_of_empty_categorical

veljanin added 2 commits October 14, 2024 12:22

Merge remote-tracking branch 'upstream/main' into fixing_pyarrow_conv…

c0d8178

…ersion_of_empty_categorical

revised testing to resolve CI errors v2

902601f

rhshadrach reviewed Oct 21, 2024

View reviewed changes

Resolve merge conflicts for code_checks docstrings...

482080d

rhshadrach requested changes Nov 7, 2024

View reviewed changes

rhshadrach reviewed Nov 7, 2024

View reviewed changes

rhshadrach added this to the 3.0 milestone Nov 7, 2024

github-actions bot added the Stale label Dec 8, 2024

rhshadrach closed this Dec 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handling conversion of empty categorical with dtype_backend='pyarrow' #59935

Handling conversion of empty categorical with dtype_backend='pyarrow' #59935

veljanin commented Oct 2, 2024

rhshadrach left a comment

rhshadrach Oct 2, 2024 •

edited

Loading

veljanin Oct 3, 2024

rhshadrach Oct 2, 2024

rhshadrach Oct 2, 2024

rhshadrach Oct 2, 2024

veljanin Oct 3, 2024

veljanin commented Oct 3, 2024

rhshadrach commented Oct 3, 2024

veljanin commented Oct 4, 2024

yuanx749 Oct 5, 2024

rhshadrach Oct 5, 2024

rhshadrach commented Oct 7, 2024

yuanx749 left a comment

yuanx749 Oct 9, 2024

veljanin commented Oct 10, 2024

rhshadrach commented Oct 12, 2024

rhshadrach Oct 21, 2024

veljanin Nov 6, 2024

rhshadrach Nov 7, 2024

rhshadrach Nov 7, 2024

rhshadrach Nov 7, 2024

rhshadrach left a comment

github-actions bot commented Dec 8, 2024

rhshadrach commented Dec 8, 2024

		ser2 = pd.Series(pd.Series(pd.Categorical([None] * 5, categories=["S1", "S2"])))
		assert (ser2.cat.categories == ["S1", "S2"]).all(), "Series categories are not empty"

	- Bug in :func:`convert_dtypes` with ``dtype_backend='pyarrow'`` parameter where empty categorical series raise error or get converted to null[pyarrow] (:issue:`59934`)
	- Bug in :meth:`Series.convert_dtypes` with ``dtype_backend='pyarrow'`` parameter where empty categorical series raise error or get converted to null[pyarrow] (:issue:`59934`)

	def test_convert_empty_categorical_to_pyarrow(self):
	def test_convert_empty_categorical_to_pyarrow(self):
	pytest.importorskip("pyarrow")

		assert converted1.dtype == "category", "Series dtype is not 'category'"
		assert converted1.cat.categories.empty, "Series categories are not empty"

Handling conversion of empty categorical with dtype_backend='pyarrow' #59935

Handling conversion of empty categorical with dtype_backend='pyarrow' #59935

Conversation

veljanin commented Oct 2, 2024

rhshadrach left a comment

Choose a reason for hiding this comment

rhshadrach Oct 2, 2024 • edited Loading

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

veljanin commented Oct 3, 2024

rhshadrach commented Oct 3, 2024

veljanin commented Oct 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rhshadrach commented Oct 7, 2024

yuanx749 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

veljanin commented Oct 10, 2024

rhshadrach commented Oct 12, 2024

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

rhshadrach left a comment

Choose a reason for hiding this comment

github-actions bot commented Dec 8, 2024

rhshadrach commented Dec 8, 2024

rhshadrach Oct 2, 2024 •

edited

Loading