[Python] pyarrow.array silently rewrites dictionary types to use signed integers for indexes

[spenczar]

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

If you define a dictionary type to use an unsigned integer for its index type, and explicitly pass it to pyarrow.array, it'll get ignored and you'll find yourself with a signed index instead.

import pyarrow as pa
dictionary_type = pa.dictionary(pa.uint64(), pa.string())
print(pa.array(['x'], dictionary_type).type)
# dictionary<values=string, indices=int64, ordered=0>

Similarly for uint32:

import pyarrow as pa
dictionary_type = pa.dictionary(pa.uint32(), pa.string())
print(pa.array(['x'], dictionary_type).type)
# dictionary<values=string, indices=int32, ordered=0>

Component(s)

Python

[jorisvandenbossche]

Thanks for the report. I think the problem is that in the python->arrow conversion (python_to_arrow.cc), it is using a DictionaryBuilder under the hood, which is created with:

arrow/cpp/src/arrow/builder.cc

Lines 312 to 320 in 9b6be29

	Status MakeDictionaryBuilder(MemoryPool* pool, const std::shared_ptr<DataType>& type,
	const std::shared_ptr<Array>& dictionary,
	std::unique_ptr<ArrayBuilder>* out) {
	const auto& dict_type = static_cast<const DictionaryType&>(*type);
	DictionaryBuilderCase visitor = {
	pool, dict_type.index_type(), dict_type.value_type(),
	dictionary, /*exact_index_type=*/false, out};
	return visitor.Make();
	}

This is passing exact_index_type = False, and that essentially means that it will use an adaptive int builder (that starts with the bitwidth size you specified, but can still grow eg from int32 to int64 if needed).

Maybe one way to fix the signed vs unsigned change is to let it use a AdaptiveIntBuilder vs AdaptiveUIntBuilder, depending on the signedness of the original index type. That would preserve the signedness, but keep the ability to let the bitwidth grow if necessary to convert the data to a dictionary type.

[mkzung]

Ran into this from the narwhals side: its Categorical maps to dictionary(uint32, string), so building one from Python values fails there. I opened #50474 before I spotted this issue, and I've closed that as a duplicate of this one.

One thing that might be worth adding here: it isn't only the signedness that gets dropped. The index type is really treated as a starting bit width, so even a signed type stops being honoured once the cardinality outgrows it:

>>> import pyarrow as pa, string, itertools
>>> vals = ["".join(p) for p in itertools.islice(itertools.product(string.ascii_lowercase, repeat=2), 200)]
>>> pa.array(["a", "b", "a"], type=pa.dictionary(pa.int8(), pa.string())).type.index_type
DataType(int8)
>>> pa.array(vals, type=pa.dictionary(pa.int8(), pa.string())).type.index_type
DataType(int16)

That falls out of the same place @jorisvandenbossche pointed at: MakeDictionaryBuilder passes exact_index_type=false, and DictionaryBuilderCase::CreateFor() reduces the requested type to index_type->byte_width() and hands that to AdaptiveIntBuilder as a starting width. So the signedness is lost and the width is only a hint.

I'd like to try the fix along the lines Joris suggested (an adaptive unsigned index builder for unsigned index types, keeping the width adaptive). Two things I hit while reading around it:

• AdaptiveUIntBuilder's constructors don't take the alignment argument that AdaptiveIntBuilder's do, so DictionaryBuilderBase<AdaptiveUIntBuilder, T> doesn't compile as it stands.
• DictionaryConverter in util/converter.h hardcodes DictionaryBuilder<ValueType> and checked_casts to it, and that trait is shared with the R binding, so it looks like it needs an index-builder template parameter rather than a local change.

Happy to pick this up if nobody else is on it.

Versions: pyarrow 23.0.1, Python 3.10, macOS arm64.

[mkzung]

take

[mkzung]

I had a go at this and landed somewhere different from what I wrote above.

The AdaptiveUIntBuilder route does not work. It makes MakeDictionaryBuilder return a different builder class, and util/converter.h, json/from_string.cc and the R binding all cast the result to DictionaryBuilder<T>. TestDictionaryUnifier.ChunkedArrayNestedDict, which builds a dictionary(uint32(), utf8()), dies on a checked_pointer_cast DCHECK as soon as you try it.

What works is keeping one builder class and preserving the requested signedness where the index type is reported. Indices are non-negative and the signed and unsigned types of a given width have the same layout, so it is value-preserving and free, and the width stays adaptive. Nothing outside builder_dict.h and builder.cc has to change, so R is fixed rather than broken.

One consequence: dictionary(uint64(), ...) then really does produce uint64 indices, and arrow_to_pandas.cc refuses to convert those, so to_pandas() starts raising where it used to work by accident on int32 indices. uint8, uint16 and uint32 are unaffected, and take, filter, cast, IPC, Parquet, the compute kernels and Acero all keep working.

PR shortly.

[pitrou]

Issue resolved by pull request 50475
#50475