Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix arrow groupby na #60777

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
+19 −1
Open

Fix arrow groupby na #60777

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
96ed248
BUG: Fix factorize to ensure proper use of null_encoding parameter
asharmalik19 Jan 20, 2025
24f5ac7
DOC: Add whatsnew entry for dictionary array NA handling fix
asharmalik19 Jan 23, 2025
c1ad98b
BUG: Fix factorize to ensure proper use of null_encoding parameter an…
asharmalik19 Jan 27, 2025
6ca56d0
DOC: Improve rst file and test case comments for arrow groupby NA fix
asharmalik19 Jan 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions doc/source/whatsnew/v3.0.0.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -790,6 +790,7 @@ ExtensionArray
^^^^^^^^^^^^^^
- Bug in :class:`Categorical` when constructing with an :class:`Index` with :class:`ArrowDtype` (:issue:`60563`)
- Bug in :meth:`.arrays.ArrowExtensionArray.__setitem__` which caused wrong behavior when using an integer array with repeated values as a key (:issue:`58530`)
- Bug in :meth:`ArrowExtensionArray.factorize` where NA values were dropped when input was dictionary-encoded even when dropna was set to False(:issue:`60567`)
- Bug in :meth:`api.types.is_datetime64_any_dtype` where a custom :class:`ExtensionDtype` would return ``False`` for array-likes (:issue:`57055`)
- Bug in comparison between object with :class:`ArrowDtype` and incompatible-dtyped (e.g. string vs bool) incorrectly raising instead of returning all-``False`` (for ``==``) or all-``True`` (for ``!=``) (:issue:`59505`)
- Bug in constructing pandas data structures when passing into ``dtype`` a string of the type followed by ``[pyarrow]`` while PyArrow is not installed would raise ``NameError`` rather than ``ImportError`` (:issue:`57928`)
Expand Down
7 changes: 6 additions & 1 deletion pandas/core/arrays/arrow/array.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1208,7 +1208,12 @@ def factorize(
data = data.cast(pa.int64())

if pa.types.is_dictionary(data.type):
encoded = data
if null_encoding == "encode":
# dictionary encode does nothing if an already encoded array is given
data = data.cast(data.type.value_type)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use dictionary_decode() instead?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dictionary_decode() works on the DictionaryArray while the input to factorize() function is a ChunkedArray. So the chunked array first needs to be combined into a dictionary array and then the dictionary_decode() can be used on it which i think is more inefficient as compared to simply casting the array to its underlying data type.

encoded = data.dictionary_encode(null_encoding=null_encoding)
else:
encoded = data
else:
encoded = data.dictionary_encode(null_encoding=null_encoding)
if encoded.length() == 0:
Expand Down
12 changes: 12 additions & 0 deletions pandas/tests/extension/test_arrow.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3329,6 +3329,18 @@ def test_factorize_chunked_dictionary():
tm.assert_index_equal(res_uniques, exp_uniques)


def test_factorize_dictionary_with_na():
# GH#60567
arr = pd.array(
["a1", pd.NA], dtype=ArrowDtype(pa.dictionary(pa.int32(), pa.utf8()))
)
indices, uniques = arr.factorize(use_na_sentinel=False)
expected_indices = np.array([0, 1], dtype=np.intp)
expected_uniques = pd.array(["a1", None], dtype=ArrowDtype(pa.string()))
tm.assert_numpy_array_equal(indices, expected_indices)
tm.assert_extension_array_equal(uniques, expected_uniques)


def test_dictionary_astype_categorical():
# GH#56672
arrs = [
Expand Down