[Python] Function 'dictionary_encode' fails with DictionaryArray input (for compute kernel / ChunkedArray method)

[randolf-scholz]

Describe the bug, including details regarding any error messages, version, and platform.

dictionary_encode should probably just return the data as-is if it is already encoded as a dictionary.

Component(s)

Python

[randolf-scholz]

MWE:

import pyarrow as pa

dict_array = pa.array(["banana", "orange"]).dictionary_encode()
dict_array.dictionary_encode()  # ✔
pa.compute.dictionary_encode(dict_array)  # ✘ ArrowNotImplementedError

[randolf-scholz]

Using chunked arrays, array.dictionary_encode() breaks as well:

import pyarrow as pa

dict_array = pa.chunked_array([["banana", "orange"], ["orange", "apple"]]).dictionary_encode()
dict_array.dictionary_encode()  # ✘ ArrowNotImplementedError
pa.compute.dictionary_encode(dict_array)  # ✘ ArrowNotImplementedError

[danepitkin]

Honestly, it might be better to keep the error and remove support for the python DictionaryArray.dictionary_encode() method. The errors being thrown all come from the same compute API in the native C++ implementation. The python method is literally just

    def dictionary_encode(self):
        return self

[jorisvandenbossche]

Indeed, and the other dictionary_encode() methods (on base class Array and on ChunkedArray) call the compute kernel, and hence give this error. Doing that for DictionaryArray.dictionary_encode as well sounds fine to me.

If we want to have support for passing a DictionaryArray to the "dictionary_encode" kernel generally, this is done in C++, and would require registering an extra version of this kernel (as a no-op for dict typed input). It seems there is actually a comment about this:

arrow/cpp/src/arrow/compute/kernels/vector_hash.cc

Lines 810 to 817 in e488942

	auto dict_encode = std::make_shared<VectorFunction>(
	"dictionary_encode", Arity::Unary(), dictionary_encode_doc,
	GetDefaultDictionaryEncodeOptions());
	AddHashKernels<DictEncodeAction>(dict_encode.get(), base, DictEncodeOutput);
	// Calling dictionary_encode on dictionary input not supported, but if it
	// ends up being needed (or convenience), a kernel could be added to make it
	// a no-op

[randolf-scholz]

One case when this bug appears in the wild is when trying to pivot a pandas.DataFrame that has been loaded with the pyarrow backend:

import pandas as pd

df = (
    pd.DataFrame([("A", 1), ("B", 2), ("C", 3)], columns=["var", "val"])
    .astype({"var": "string", "val": "float32"})
    .astype({"var": "category", "val": "float32"})
)

# write and reload as parquet with pyarrow backend
df.to_parquet("demo.parquet")
df = pd.read_parquet("demo.parquet", dtype_backend="pyarrow")
print(df.dtypes)  # var is now dictionary[int32,string]

df.pivot(columns=["var"], values=["val"])  # ✘ ArrowNotImplementedError

[jorisvandenbossche]

@randolf-scholz that's something that pandas can fix on their side in the meantime, so I would report that example over there (if you haven't done that yet)

[jorisvandenbossche]

But it is certainly true that for convenience (to avoid having to check if the input array is already dict encoded before calling dictionary_encode), it might be nice that pyarrow.compute.dictionary_encode would just be a no-op for dictionary encoded arrays.

[ddutt]

I see this problem even for pandas frames with groupby followed by any operation such as nunique, count etc.

[jorisvandenbossche]

@ddutt please report that to pandas

[randolf-scholz]

Imo, the preferred solution here would be:

1. keep .dictionary_encode as an identity function (LSP!)
2. add the no-op C++ kernel
3. Enable conversion to dictionary via cast (why is there a separate dictionary_encode function to begin with?) - but this is a separate issue.

Functions like dictionary_encode or cast should be idempotent.