GH-42018: [Python] Add NumPy StringDType -> Arrow support

[alippai]

Rationale for this change

Implement #42018

What changes are included in this PR?

Conversion in numpy->arrow direction with multiple string types

Are these changes tested?

Two basic conversion tests added and one boundary that only string(), large_string(), string_view() is supported.

Are there any user-facing changes?

Yes, adds support to numpy.StringDType as source

cc @jorisvandenbossche as he opened the original issue

This PR was created using GPT-5.6-Sol-xhigh, every line read & reviewed by me.

• GitHub Issue: [Python] Conversion to/from numpy 2.0+ new StringDType #42018

[alippai]

@ngoldbaum can I ask you for review?

[alippai]

The commits are in complexity & speed order. They are supposed to be reviewable commit-by-commit.

[ngoldbaum]

I'll do a pass over this next week. Thank you for moving this forward.

[ngoldbaum]

The two CI failures are unrelated, I think.

Before this can be merged, you should also explicitly mention this feature in docs/source/python/numpy.rst. Right now it just says arrow supports "structured dtypes or strings", but NumPy's string support has grown more complicated since that was written and it's worth updating now to say something like "structured dtypes and both fixed-width, and variable-width strings". Maybe do a further pass to see if there are other spots that could mention this.

I carefully reviewed this for lock discipline and didn't spot any bugs. I do not see any places where possibly-blocking APIs are called while the allocator lock is held. Also the way this is currently structured, it can't conflict with the GIL because the conversion happens in code that explicitly does not hold the GIL.

I have some minor comments below but I think this is in mostly good shape from the perspective of NumPy C API use.

[ngoldbaum]

Suggested change

	constchar* data = PyArray_BYTES(arr_);
	auto* allocator =
	NpyString_acquire_allocator(reinterpret_cast<PyArray_StringDTypeObject*>(dtype_));
	std::unique_ptr<npy_string_allocator, decltype(&NpyString_release_allocator)>
	allocator_guard(allocator, &NpyString_release_allocator);
	constbool has_mask = mask_ != nullptr;
	Ndarray1DIndexer<uint8_t> mask_values;
	if (has_mask) {
	mask_values = Ndarray1DIndexer<uint8_t>(mask_);
	}
	constexprint64_tkBatchSize = 4096;
	// NpyString_load returns borrowed views that remain valid while the allocator is
	// locked.
	constint64_t batch_capacity = std::min(length_, kBatchSize);
	std::vector<std::string_view> values(batch_capacity);
	std::vector<uint8_t> valid(batch_capacity);
	npy_static_string value{};
	for (int64_t offset = 0; offset < length_; offset += kBatchSize) {
	constchar* data = PyArray_BYTES(arr_);
	constbool has_mask = mask_ != nullptr;
	Ndarray1DIndexer<uint8_t> mask_values;
	if (has_mask) {
	mask_values = Ndarray1DIndexer<uint8_t>(mask_);
	}
	constexprint64_tkBatchSize = 4096;
	// NpyString_load returns borrowed views that remain valid while the allocator is
	// locked.
	constint64_t batch_capacity = std::min(length_, kBatchSize);
	std::vector<std::string_view> values(batch_capacity);
	std::vector<uint8_t> valid(batch_capacity);
	npy_static_string value{};
	// Hold the allocator lock for the whole conversion to ensure a consistent snapshot
	auto* allocator =
	NpyString_acquire_allocator(reinterpret_cast<PyArray_StringDTypeObject*>(dtype_));
	std::unique_ptr<npy_string_allocator, decltype(&NpyString_release_allocator)>
	allocator_guard(allocator, &NpyString_release_allocator);
	for (int64_t offset = 0; offset < length_; offset += kBatchSize) {

This way you hold the allocator lock for the minimum amount of code, I also added a comment explaining that there is a mutex here so a reader knows to watch out for deadlock risks.

[ngoldbaum]

Suggested change

	return VisitStringDType(builder);
	// Acquires a lock so this must not be moved inside the gil_lock section below
	return VisitStringDType(builder);

[ngoldbaum]

Might be nice to see the type that was passed in:

Suggested change

	"NumPy StringDType can only be converted to Arrow string types");
	"NumPy StringDType can only be converted to Arrow string types, got",
	type_->ToString());

[ngoldbaum]

My AI model thinks that doing this has an unintended, untested side effect: pa.array([np.array([...], dtype="T")]) now infers as a list of strings and converts through the per-element sequence fallback in python_to_arrow.cc:

arrow/python/pyarrow/src/arrow/python/python_to_arrow.cc

Lines 945 to 947 in 54ea6d5

	if (PyArray_DESCR(ndarray)->type_num != NUMPY_TYPE) { \
	returnthis->value_converter_->Extend(value, size); \
	} \

That's fine, it just needs a test to cover it.

[ngoldbaum]

This is a public header so this shouldn't be deleted. Deleting this also caused churn in this PR in other compilation units, which newly add #include <numeric> to work around this. The same could happen in user code including this header.

[ngoldbaum]

Suggested change

	@pytest.mark.parametrize('string_type', [pa.large_string(), pa.string_view()])
	@pytest.mark.parametrize('string_type', [None, pa.large_string(), pa.string_view()])

Let's include the default string() type too.

[ngoldbaum]

Why is the output missing sentinel None for all input na_object choices, in particular for strings?

Note that this also disagrees with NumPy:

>>> np.array(["hello", "__placeholder__", "world"], dtype="T").tolist()
['hello', '__placeholder__', 'world']

Is there a reason why you can't more faithfully translate NumPy's string missing data semantics?

[ngoldbaum]

The first test improves coverage for your changes to ChunkedBinaryBuilder. The second test covers the untested case I referred to in my comment above where a list of ndarrays gains new inference behavior.

Suggested change

	@pytest.mark.numpy
	deftest_array_from_numpy_string_dtype_chunking(numpy_string_dtype):
	# Three 6 MiB values in one batch must split across the 16 MiB
	# per-chunk limit of the string() path.
	values= ["x"* (6*1024*1024)] *3
	arr=np.array(values, dtype=numpy_string_dtype())
	result=pa.array(arr, type=pa.string())
	assertisinstance(result, pa.ChunkedArray)
	assertresult.num_chunks==2
	result.validate(full=True)
	assertresult.to_pylist() ==values
	@pytest.mark.numpy
	deftest_array_from_list_of_numpy_string_dtype_arrays(numpy_string_dtype):
	values= [["a", "bb"], ["ccc"]]
	arrays= [np.array(v, dtype=numpy_string_dtype()) forvinvalues]
	result=pa.array(arrays)
	assertresult.type==pa.list_(pa.string())
	assertresult.to_pylist() ==values

[jorisvandenbossche]

@alippai your previous PR #48391 did not need changes to the Builder code, while now there seems to be a significant addition there. Can you provide some context regarding the different implementation strategy?

[jorisvandenbossche]

Inline comment of @ngoldbaum (moving it here it make it more prominent and not hidden in a potentially outdated/collapsed review comment):

Why is the output missing sentinel None for all input na_object choices, in particular for strings?

Note that this also disagrees with NumPy:

>>> np.array(["hello", "__placeholder__", "world"], dtype="T").tolist()
['hello', '__placeholder__', 'world']

Is there a reason why you can't more faithfully translate NumPy's string missing data semantics?

I think this is the main API design question to discuss: the numpy StringDtype has a configurable missing value sentinel model, while Arrow has no such option and just has nulls (through the bitmask).
So that means it is not really possible to fully "faithfully" translate those missing semantics in the numpy->arrow conversion. Either:

• we use the actual placeholder value (or at least if it is a string) as the resulting string value in the Arrow string array. That preserves the value, but looses the fact that it is missing
• we translate the placeholder values to nulls (what this PR currently does). That preserves the missingness, but looses the information about the original placeholder value used.

So we always loose something (unless we would create an extension type ..). Personally, my feeling is that preserving "missingness" is the most relevant.

FWIW, this also means that a fully faithful roundtrip (once the conversion from arrow -> numpy exists) is also not possible out of the box, only if the user specifies the resulting numpy dtype (which has the information about which placeholder to use)

[alippai]

@alippai your previous PR #48391 did not need changes to the Builder code, while now there seems to be a significant addition there. Can you provide some context regarding the different implementation strategy?

@jorisvandenbossche this PR has a similar simpler version as the first commit. I opportunistically added a few batching, performance optimizations in the following 2-3rd commits (without benchmarks, I don’t know how trivial is this considered and I don’t have a representative machine for this right now).

[ngoldbaum]

this PR has a similar simpler version as the first commit

Maybe as a first pass you could try to just merge the simpler version. Then later in future PRs you could add the optimizations, along with benchmarks to justify them.

[ngoldbaum]

@alippai gentle ping here. Just in case it's helpful: I'm happy to take over shepherding this feature as I can work on it under funded time.

[alippai]

@ngoldbaum I can work on this later this weekend only. Feel free to either take the first commit or start from scratch

[ngoldbaum]

I went ahead and opened #51157 which I marked as superseding this one.

[alippai]

Rationale for this change

Implement #42018

What changes are included in this PR?

Conversion in numpy->arrow direction with multiple string types

Are these changes tested?

Two basic conversion tests added and one boundary that only string(), large_string(), string_view() is supported.

Are there any user-facing changes?

Yes, adds support to numpy.StringDType as source

cc @jorisvandenbossche as he opened the original issue

This PR was created using GPT-5.6-Sol-xhigh, every line read & reviewed by me.

• GitHub Issue: [Python] Conversion to/from numpy 2.0+ new StringDType #42018

[alippai]

@ngoldbaum can I ask you for review?

[alippai]

The commits are in complexity & speed order. They are supposed to be reviewable commit-by-commit.

[ngoldbaum]

I'll do a pass over this next week. Thank you for moving this forward.

[ngoldbaum]

The two CI failures are unrelated, I think.

Before this can be merged, you should also explicitly mention this feature in docs/source/python/numpy.rst. Right now it just says arrow supports "structured dtypes or strings", but NumPy's string support has grown more complicated since that was written and it's worth updating now to say something like "structured dtypes and both fixed-width, and variable-width strings". Maybe do a further pass to see if there are other spots that could mention this.

I carefully reviewed this for lock discipline and didn't spot any bugs. I do not see any places where possibly-blocking APIs are called while the allocator lock is held. Also the way this is currently structured, it can't conflict with the GIL because the conversion happens in code that explicitly does not hold the GIL.

I have some minor comments below but I think this is in mostly good shape from the perspective of NumPy C API use.

[ngoldbaum]

Suggested change

	constchar* data = PyArray_BYTES(arr_);
	auto* allocator =
	NpyString_acquire_allocator(reinterpret_cast<PyArray_StringDTypeObject*>(dtype_));
	std::unique_ptr<npy_string_allocator, decltype(&NpyString_release_allocator)>
	allocator_guard(allocator, &NpyString_release_allocator);
	constbool has_mask = mask_ != nullptr;
	Ndarray1DIndexer<uint8_t> mask_values;
	if (has_mask) {
	mask_values = Ndarray1DIndexer<uint8_t>(mask_);
	}
	constexprint64_tkBatchSize = 4096;
	// NpyString_load returns borrowed views that remain valid while the allocator is
	// locked.
	constint64_t batch_capacity = std::min(length_, kBatchSize);
	std::vector<std::string_view> values(batch_capacity);
	std::vector<uint8_t> valid(batch_capacity);
	npy_static_string value{};
	for (int64_t offset = 0; offset < length_; offset += kBatchSize) {
	constchar* data = PyArray_BYTES(arr_);
	constbool has_mask = mask_ != nullptr;
	Ndarray1DIndexer<uint8_t> mask_values;
	if (has_mask) {
	mask_values = Ndarray1DIndexer<uint8_t>(mask_);
	}
	constexprint64_tkBatchSize = 4096;
	// NpyString_load returns borrowed views that remain valid while the allocator is
	// locked.
	constint64_t batch_capacity = std::min(length_, kBatchSize);
	std::vector<std::string_view> values(batch_capacity);
	std::vector<uint8_t> valid(batch_capacity);
	npy_static_string value{};
	// Hold the allocator lock for the whole conversion to ensure a consistent snapshot
	auto* allocator =
	NpyString_acquire_allocator(reinterpret_cast<PyArray_StringDTypeObject*>(dtype_));
	std::unique_ptr<npy_string_allocator, decltype(&NpyString_release_allocator)>
	allocator_guard(allocator, &NpyString_release_allocator);
	for (int64_t offset = 0; offset < length_; offset += kBatchSize) {

This way you hold the allocator lock for the minimum amount of code, I also added a comment explaining that there is a mutex here so a reader knows to watch out for deadlock risks.

[ngoldbaum]

Suggested change

	return VisitStringDType(builder);
	// Acquires a lock so this must not be moved inside the gil_lock section below
	return VisitStringDType(builder);

[ngoldbaum]

Might be nice to see the type that was passed in:

Suggested change

	"NumPy StringDType can only be converted to Arrow string types");
	"NumPy StringDType can only be converted to Arrow string types, got",
	type_->ToString());

[ngoldbaum]

My AI model thinks that doing this has an unintended, untested side effect: pa.array([np.array([...], dtype="T")]) now infers as a list of strings and converts through the per-element sequence fallback in python_to_arrow.cc:

arrow/python/pyarrow/src/arrow/python/python_to_arrow.cc

Lines 945 to 947 in 54ea6d5

	if (PyArray_DESCR(ndarray)->type_num != NUMPY_TYPE) { \
	returnthis->value_converter_->Extend(value, size); \
	} \

That's fine, it just needs a test to cover it.

[ngoldbaum]

This is a public header so this shouldn't be deleted. Deleting this also caused churn in this PR in other compilation units, which newly add #include <numeric> to work around this. The same could happen in user code including this header.

[ngoldbaum]

Suggested change

	@pytest.mark.parametrize('string_type', [pa.large_string(), pa.string_view()])
	@pytest.mark.parametrize('string_type', [None, pa.large_string(), pa.string_view()])

Let's include the default string() type too.

[ngoldbaum]

Why is the output missing sentinel None for all input na_object choices, in particular for strings?

Note that this also disagrees with NumPy:

>>> np.array(["hello", "__placeholder__", "world"], dtype="T").tolist()
['hello', '__placeholder__', 'world']

Is there a reason why you can't more faithfully translate NumPy's string missing data semantics?

[ngoldbaum]

The first test improves coverage for your changes to ChunkedBinaryBuilder. The second test covers the untested case I referred to in my comment above where a list of ndarrays gains new inference behavior.

Suggested change

	@pytest.mark.numpy
	deftest_array_from_numpy_string_dtype_chunking(numpy_string_dtype):
	# Three 6 MiB values in one batch must split across the 16 MiB
	# per-chunk limit of the string() path.
	values= ["x"* (6*1024*1024)] *3
	arr=np.array(values, dtype=numpy_string_dtype())
	result=pa.array(arr, type=pa.string())
	assertisinstance(result, pa.ChunkedArray)
	assertresult.num_chunks==2
	result.validate(full=True)
	assertresult.to_pylist() ==values
	@pytest.mark.numpy
	deftest_array_from_list_of_numpy_string_dtype_arrays(numpy_string_dtype):
	values= [["a", "bb"], ["ccc"]]
	arrays= [np.array(v, dtype=numpy_string_dtype()) forvinvalues]
	result=pa.array(arrays)
	assertresult.type==pa.list_(pa.string())
	assertresult.to_pylist() ==values

[jorisvandenbossche]

@alippai your previous PR #48391 did not need changes to the Builder code, while now there seems to be a significant addition there. Can you provide some context regarding the different implementation strategy?

[jorisvandenbossche]

Inline comment of @ngoldbaum (moving it here it make it more prominent and not hidden in a potentially outdated/collapsed review comment):

Why is the output missing sentinel None for all input na_object choices, in particular for strings?

Note that this also disagrees with NumPy:

>>> np.array(["hello", "__placeholder__", "world"], dtype="T").tolist()
['hello', '__placeholder__', 'world']

Is there a reason why you can't more faithfully translate NumPy's string missing data semantics?

I think this is the main API design question to discuss: the numpy StringDtype has a configurable missing value sentinel model, while Arrow has no such option and just has nulls (through the bitmask).
So that means it is not really possible to fully "faithfully" translate those missing semantics in the numpy->arrow conversion. Either:

• we use the actual placeholder value (or at least if it is a string) as the resulting string value in the Arrow string array. That preserves the value, but looses the fact that it is missing
• we translate the placeholder values to nulls (what this PR currently does). That preserves the missingness, but looses the information about the original placeholder value used.

So we always loose something (unless we would create an extension type ..). Personally, my feeling is that preserving "missingness" is the most relevant.

FWIW, this also means that a fully faithful roundtrip (once the conversion from arrow -> numpy exists) is also not possible out of the box, only if the user specifies the resulting numpy dtype (which has the information about which placeholder to use)

[alippai]

@alippai your previous PR #48391 did not need changes to the Builder code, while now there seems to be a significant addition there. Can you provide some context regarding the different implementation strategy?

@jorisvandenbossche this PR has a similar simpler version as the first commit. I opportunistically added a few batching, performance optimizations in the following 2-3rd commits (without benchmarks, I don’t know how trivial is this considered and I don’t have a representative machine for this right now).

[ngoldbaum]

this PR has a similar simpler version as the first commit

Maybe as a first pass you could try to just merge the simpler version. Then later in future PRs you could add the optimizations, along with benchmarks to justify them.

[ngoldbaum]

@alippai gentle ping here. Just in case it's helpful: I'm happy to take over shepherding this feature as I can work on it under funded time.

[alippai]

@ngoldbaum I can work on this later this weekend only. Feel free to either take the first commit or start from scratch

[ngoldbaum]

I went ahead and opened #51157 which I marked as superseding this one.

[alippai]

Rationale for this change

Implement #42018

What changes are included in this PR?

Conversion in numpy->arrow direction with multiple string types

Are these changes tested?

Two basic conversion tests added and one boundary that only string(), large_string(), string_view() is supported.

Are there any user-facing changes?

Yes, adds support to numpy.StringDType as source

cc @jorisvandenbossche as he opened the original issue

This PR was created using GPT-5.6-Sol-xhigh, every line read & reviewed by me.

• GitHub Issue: [Python] Conversion to/from numpy 2.0+ new StringDType #42018

[alippai]

@ngoldbaum can I ask you for review?

[alippai]

The commits are in complexity & speed order. They are supposed to be reviewable commit-by-commit.

[ngoldbaum]

I'll do a pass over this next week. Thank you for moving this forward.

[ngoldbaum]

The two CI failures are unrelated, I think.

Before this can be merged, you should also explicitly mention this feature in docs/source/python/numpy.rst. Right now it just says arrow supports "structured dtypes or strings", but NumPy's string support has grown more complicated since that was written and it's worth updating now to say something like "structured dtypes and both fixed-width, and variable-width strings". Maybe do a further pass to see if there are other spots that could mention this.

I carefully reviewed this for lock discipline and didn't spot any bugs. I do not see any places where possibly-blocking APIs are called while the allocator lock is held. Also the way this is currently structured, it can't conflict with the GIL because the conversion happens in code that explicitly does not hold the GIL.

I have some minor comments below but I think this is in mostly good shape from the perspective of NumPy C API use.

[ngoldbaum]

Suggested change

	constchar* data = PyArray_BYTES(arr_);
	auto* allocator =
	NpyString_acquire_allocator(reinterpret_cast<PyArray_StringDTypeObject*>(dtype_));
	std::unique_ptr<npy_string_allocator, decltype(&NpyString_release_allocator)>
	allocator_guard(allocator, &NpyString_release_allocator);
	constbool has_mask = mask_ != nullptr;
	Ndarray1DIndexer<uint8_t> mask_values;
	if (has_mask) {
	mask_values = Ndarray1DIndexer<uint8_t>(mask_);
	}
	constexprint64_tkBatchSize = 4096;
	// NpyString_load returns borrowed views that remain valid while the allocator is
	// locked.
	constint64_t batch_capacity = std::min(length_, kBatchSize);
	std::vector<std::string_view> values(batch_capacity);
	std::vector<uint8_t> valid(batch_capacity);
	npy_static_string value{};
	for (int64_t offset = 0; offset < length_; offset += kBatchSize) {
	constchar* data = PyArray_BYTES(arr_);
	constbool has_mask = mask_ != nullptr;
	Ndarray1DIndexer<uint8_t> mask_values;
	if (has_mask) {
	mask_values = Ndarray1DIndexer<uint8_t>(mask_);
	}
	constexprint64_tkBatchSize = 4096;
	// NpyString_load returns borrowed views that remain valid while the allocator is
	// locked.
	constint64_t batch_capacity = std::min(length_, kBatchSize);
	std::vector<std::string_view> values(batch_capacity);
	std::vector<uint8_t> valid(batch_capacity);
	npy_static_string value{};
	// Hold the allocator lock for the whole conversion to ensure a consistent snapshot
	auto* allocator =
	NpyString_acquire_allocator(reinterpret_cast<PyArray_StringDTypeObject*>(dtype_));
	std::unique_ptr<npy_string_allocator, decltype(&NpyString_release_allocator)>
	allocator_guard(allocator, &NpyString_release_allocator);
	for (int64_t offset = 0; offset < length_; offset += kBatchSize) {

This way you hold the allocator lock for the minimum amount of code, I also added a comment explaining that there is a mutex here so a reader knows to watch out for deadlock risks.

[ngoldbaum]

Suggested change

	return VisitStringDType(builder);
	// Acquires a lock so this must not be moved inside the gil_lock section below
	return VisitStringDType(builder);

[ngoldbaum]

Might be nice to see the type that was passed in:

Suggested change

	"NumPy StringDType can only be converted to Arrow string types");
	"NumPy StringDType can only be converted to Arrow string types, got",
	type_->ToString());

[ngoldbaum]

My AI model thinks that doing this has an unintended, untested side effect: pa.array([np.array([...], dtype="T")]) now infers as a list of strings and converts through the per-element sequence fallback in python_to_arrow.cc:

arrow/python/pyarrow/src/arrow/python/python_to_arrow.cc

Lines 945 to 947 in 54ea6d5

	if (PyArray_DESCR(ndarray)->type_num != NUMPY_TYPE) { \
	returnthis->value_converter_->Extend(value, size); \
	} \

That's fine, it just needs a test to cover it.

[ngoldbaum]

This is a public header so this shouldn't be deleted. Deleting this also caused churn in this PR in other compilation units, which newly add #include <numeric> to work around this. The same could happen in user code including this header.

[ngoldbaum]

Suggested change

	@pytest.mark.parametrize('string_type', [pa.large_string(), pa.string_view()])
	@pytest.mark.parametrize('string_type', [None, pa.large_string(), pa.string_view()])

Let's include the default string() type too.

[ngoldbaum]

Why is the output missing sentinel None for all input na_object choices, in particular for strings?

Note that this also disagrees with NumPy:

>>> np.array(["hello", "__placeholder__", "world"], dtype="T").tolist()
['hello', '__placeholder__', 'world']

Is there a reason why you can't more faithfully translate NumPy's string missing data semantics?

[ngoldbaum]

The first test improves coverage for your changes to ChunkedBinaryBuilder. The second test covers the untested case I referred to in my comment above where a list of ndarrays gains new inference behavior.

Suggested change

	@pytest.mark.numpy
	deftest_array_from_numpy_string_dtype_chunking(numpy_string_dtype):
	# Three 6 MiB values in one batch must split across the 16 MiB
	# per-chunk limit of the string() path.
	values= ["x"* (6*1024*1024)] *3
	arr=np.array(values, dtype=numpy_string_dtype())
	result=pa.array(arr, type=pa.string())
	assertisinstance(result, pa.ChunkedArray)
	assertresult.num_chunks==2
	result.validate(full=True)
	assertresult.to_pylist() ==values
	@pytest.mark.numpy
	deftest_array_from_list_of_numpy_string_dtype_arrays(numpy_string_dtype):
	values= [["a", "bb"], ["ccc"]]
	arrays= [np.array(v, dtype=numpy_string_dtype()) forvinvalues]
	result=pa.array(arrays)
	assertresult.type==pa.list_(pa.string())
	assertresult.to_pylist() ==values

[jorisvandenbossche]

@alippai your previous PR #48391 did not need changes to the Builder code, while now there seems to be a significant addition there. Can you provide some context regarding the different implementation strategy?

[jorisvandenbossche]

Inline comment of @ngoldbaum (moving it here it make it more prominent and not hidden in a potentially outdated/collapsed review comment):

Why is the output missing sentinel None for all input na_object choices, in particular for strings?

Note that this also disagrees with NumPy:

>>> np.array(["hello", "__placeholder__", "world"], dtype="T").tolist()
['hello', '__placeholder__', 'world']

Is there a reason why you can't more faithfully translate NumPy's string missing data semantics?

I think this is the main API design question to discuss: the numpy StringDtype has a configurable missing value sentinel model, while Arrow has no such option and just has nulls (through the bitmask).
So that means it is not really possible to fully "faithfully" translate those missing semantics in the numpy->arrow conversion. Either:

• we use the actual placeholder value (or at least if it is a string) as the resulting string value in the Arrow string array. That preserves the value, but looses the fact that it is missing
• we translate the placeholder values to nulls (what this PR currently does). That preserves the missingness, but looses the information about the original placeholder value used.

So we always loose something (unless we would create an extension type ..). Personally, my feeling is that preserving "missingness" is the most relevant.

FWIW, this also means that a fully faithful roundtrip (once the conversion from arrow -> numpy exists) is also not possible out of the box, only if the user specifies the resulting numpy dtype (which has the information about which placeholder to use)

[alippai]

@alippai your previous PR #48391 did not need changes to the Builder code, while now there seems to be a significant addition there. Can you provide some context regarding the different implementation strategy?

@jorisvandenbossche this PR has a similar simpler version as the first commit. I opportunistically added a few batching, performance optimizations in the following 2-3rd commits (without benchmarks, I don’t know how trivial is this considered and I don’t have a representative machine for this right now).

[ngoldbaum]

this PR has a similar simpler version as the first commit

Maybe as a first pass you could try to just merge the simpler version. Then later in future PRs you could add the optimizations, along with benchmarks to justify them.

[ngoldbaum]

@alippai gentle ping here. Just in case it's helpful: I'm happy to take over shepherding this feature as I can work on it under funded time.

[alippai]

@ngoldbaum I can work on this later this weekend only. Feel free to either take the first commit or start from scratch

[ngoldbaum]

I went ahead and opened #51157 which I marked as superseding this one.

[alippai]

Rationale for this change

Implement #42018

What changes are included in this PR?

Conversion in numpy->arrow direction with multiple string types

Are these changes tested?

Two basic conversion tests added and one boundary that only string(), large_string(), string_view() is supported.

Are there any user-facing changes?

Yes, adds support to numpy.StringDType as source

cc @jorisvandenbossche as he opened the original issue

This PR was created using GPT-5.6-Sol-xhigh, every line read & reviewed by me.

• GitHub Issue: [Python] Conversion to/from numpy 2.0+ new StringDType #42018

[alippai]

@ngoldbaum can I ask you for review?

[alippai]

The commits are in complexity & speed order. They are supposed to be reviewable commit-by-commit.

[ngoldbaum]

I'll do a pass over this next week. Thank you for moving this forward.

[ngoldbaum]

The two CI failures are unrelated, I think.

Before this can be merged, you should also explicitly mention this feature in docs/source/python/numpy.rst. Right now it just says arrow supports "structured dtypes or strings", but NumPy's string support has grown more complicated since that was written and it's worth updating now to say something like "structured dtypes and both fixed-width, and variable-width strings". Maybe do a further pass to see if there are other spots that could mention this.

I carefully reviewed this for lock discipline and didn't spot any bugs. I do not see any places where possibly-blocking APIs are called while the allocator lock is held. Also the way this is currently structured, it can't conflict with the GIL because the conversion happens in code that explicitly does not hold the GIL.

I have some minor comments below but I think this is in mostly good shape from the perspective of NumPy C API use.

[ngoldbaum]

Suggested change

	constchar* data = PyArray_BYTES(arr_);
	auto* allocator =
	NpyString_acquire_allocator(reinterpret_cast<PyArray_StringDTypeObject*>(dtype_));
	std::unique_ptr<npy_string_allocator, decltype(&NpyString_release_allocator)>
	allocator_guard(allocator, &NpyString_release_allocator);
	constbool has_mask = mask_ != nullptr;
	Ndarray1DIndexer<uint8_t> mask_values;
	if (has_mask) {
	mask_values = Ndarray1DIndexer<uint8_t>(mask_);
	}
	constexprint64_tkBatchSize = 4096;
	// NpyString_load returns borrowed views that remain valid while the allocator is
	// locked.
	constint64_t batch_capacity = std::min(length_, kBatchSize);
	std::vector<std::string_view> values(batch_capacity);
	std::vector<uint8_t> valid(batch_capacity);
	npy_static_string value{};
	for (int64_t offset = 0; offset < length_; offset += kBatchSize) {
	constchar* data = PyArray_BYTES(arr_);
	constbool has_mask = mask_ != nullptr;
	Ndarray1DIndexer<uint8_t> mask_values;
	if (has_mask) {
	mask_values = Ndarray1DIndexer<uint8_t>(mask_);
	}
	constexprint64_tkBatchSize = 4096;
	// NpyString_load returns borrowed views that remain valid while the allocator is
	// locked.
	constint64_t batch_capacity = std::min(length_, kBatchSize);
	std::vector<std::string_view> values(batch_capacity);
	std::vector<uint8_t> valid(batch_capacity);
	npy_static_string value{};
	// Hold the allocator lock for the whole conversion to ensure a consistent snapshot
	auto* allocator =
	NpyString_acquire_allocator(reinterpret_cast<PyArray_StringDTypeObject*>(dtype_));
	std::unique_ptr<npy_string_allocator, decltype(&NpyString_release_allocator)>
	allocator_guard(allocator, &NpyString_release_allocator);
	for (int64_t offset = 0; offset < length_; offset += kBatchSize) {

This way you hold the allocator lock for the minimum amount of code, I also added a comment explaining that there is a mutex here so a reader knows to watch out for deadlock risks.

[ngoldbaum]

Suggested change

	return VisitStringDType(builder);
	// Acquires a lock so this must not be moved inside the gil_lock section below
	return VisitStringDType(builder);

[ngoldbaum]

Might be nice to see the type that was passed in:

Suggested change

	"NumPy StringDType can only be converted to Arrow string types");
	"NumPy StringDType can only be converted to Arrow string types, got",
	type_->ToString());

[ngoldbaum]

My AI model thinks that doing this has an unintended, untested side effect: pa.array([np.array([...], dtype="T")]) now infers as a list of strings and converts through the per-element sequence fallback in python_to_arrow.cc:

arrow/python/pyarrow/src/arrow/python/python_to_arrow.cc

Lines 945 to 947 in 54ea6d5

	if (PyArray_DESCR(ndarray)->type_num != NUMPY_TYPE) { \
	returnthis->value_converter_->Extend(value, size); \
	} \

That's fine, it just needs a test to cover it.

[ngoldbaum]

This is a public header so this shouldn't be deleted. Deleting this also caused churn in this PR in other compilation units, which newly add #include <numeric> to work around this. The same could happen in user code including this header.

[ngoldbaum]

Suggested change

	@pytest.mark.parametrize('string_type', [pa.large_string(), pa.string_view()])
	@pytest.mark.parametrize('string_type', [None, pa.large_string(), pa.string_view()])

Let's include the default string() type too.

[ngoldbaum]

Why is the output missing sentinel None for all input na_object choices, in particular for strings?

Note that this also disagrees with NumPy:

>>> np.array(["hello", "__placeholder__", "world"], dtype="T").tolist()
['hello', '__placeholder__', 'world']

Is there a reason why you can't more faithfully translate NumPy's string missing data semantics?

[ngoldbaum]

The first test improves coverage for your changes to ChunkedBinaryBuilder. The second test covers the untested case I referred to in my comment above where a list of ndarrays gains new inference behavior.

Suggested change

	@pytest.mark.numpy
	deftest_array_from_numpy_string_dtype_chunking(numpy_string_dtype):
	# Three 6 MiB values in one batch must split across the 16 MiB
	# per-chunk limit of the string() path.
	values= ["x"* (6*1024*1024)] *3
	arr=np.array(values, dtype=numpy_string_dtype())
	result=pa.array(arr, type=pa.string())
	assertisinstance(result, pa.ChunkedArray)
	assertresult.num_chunks==2
	result.validate(full=True)
	assertresult.to_pylist() ==values
	@pytest.mark.numpy
	deftest_array_from_list_of_numpy_string_dtype_arrays(numpy_string_dtype):
	values= [["a", "bb"], ["ccc"]]
	arrays= [np.array(v, dtype=numpy_string_dtype()) forvinvalues]
	result=pa.array(arrays)
	assertresult.type==pa.list_(pa.string())
	assertresult.to_pylist() ==values

[jorisvandenbossche]

@alippai your previous PR #48391 did not need changes to the Builder code, while now there seems to be a significant addition there. Can you provide some context regarding the different implementation strategy?

[jorisvandenbossche]

Inline comment of @ngoldbaum (moving it here it make it more prominent and not hidden in a potentially outdated/collapsed review comment):

Why is the output missing sentinel None for all input na_object choices, in particular for strings?

Note that this also disagrees with NumPy:

>>> np.array(["hello", "__placeholder__", "world"], dtype="T").tolist()
['hello', '__placeholder__', 'world']

Is there a reason why you can't more faithfully translate NumPy's string missing data semantics?

I think this is the main API design question to discuss: the numpy StringDtype has a configurable missing value sentinel model, while Arrow has no such option and just has nulls (through the bitmask).
So that means it is not really possible to fully "faithfully" translate those missing semantics in the numpy->arrow conversion. Either:

• we use the actual placeholder value (or at least if it is a string) as the resulting string value in the Arrow string array. That preserves the value, but looses the fact that it is missing
• we translate the placeholder values to nulls (what this PR currently does). That preserves the missingness, but looses the information about the original placeholder value used.

So we always loose something (unless we would create an extension type ..). Personally, my feeling is that preserving "missingness" is the most relevant.

FWIW, this also means that a fully faithful roundtrip (once the conversion from arrow -> numpy exists) is also not possible out of the box, only if the user specifies the resulting numpy dtype (which has the information about which placeholder to use)

[alippai]

@alippai your previous PR #48391 did not need changes to the Builder code, while now there seems to be a significant addition there. Can you provide some context regarding the different implementation strategy?

@jorisvandenbossche this PR has a similar simpler version as the first commit. I opportunistically added a few batching, performance optimizations in the following 2-3rd commits (without benchmarks, I don’t know how trivial is this considered and I don’t have a representative machine for this right now).

[ngoldbaum]

this PR has a similar simpler version as the first commit

Maybe as a first pass you could try to just merge the simpler version. Then later in future PRs you could add the optimizations, along with benchmarks to justify them.

[ngoldbaum]

@alippai gentle ping here. Just in case it's helpful: I'm happy to take over shepherding this feature as I can work on it under funded time.

[alippai]

@ngoldbaum I can work on this later this weekend only. Feel free to either take the first commit or start from scratch

[ngoldbaum]

I went ahead and opened #51157 which I marked as superseding this one.

[alippai]

Rationale for this change

Implement #42018

What changes are included in this PR?

Conversion in numpy->arrow direction with multiple string types

Are these changes tested?

Two basic conversion tests added and one boundary that only string(), large_string(), string_view() is supported.

Are there any user-facing changes?

Yes, adds support to numpy.StringDType as source

cc @jorisvandenbossche as he opened the original issue

This PR was created using GPT-5.6-Sol-xhigh, every line read & reviewed by me.

• GitHub Issue: [Python] Conversion to/from numpy 2.0+ new StringDType #42018

[alippai]

@ngoldbaum can I ask you for review?

[alippai]

The commits are in complexity & speed order. They are supposed to be reviewable commit-by-commit.

[ngoldbaum]

I'll do a pass over this next week. Thank you for moving this forward.

[ngoldbaum]

The two CI failures are unrelated, I think.

Before this can be merged, you should also explicitly mention this feature in docs/source/python/numpy.rst. Right now it just says arrow supports "structured dtypes or strings", but NumPy's string support has grown more complicated since that was written and it's worth updating now to say something like "structured dtypes and both fixed-width, and variable-width strings". Maybe do a further pass to see if there are other spots that could mention this.

I carefully reviewed this for lock discipline and didn't spot any bugs. I do not see any places where possibly-blocking APIs are called while the allocator lock is held. Also the way this is currently structured, it can't conflict with the GIL because the conversion happens in code that explicitly does not hold the GIL.

I have some minor comments below but I think this is in mostly good shape from the perspective of NumPy C API use.

[ngoldbaum]

Suggested change

	constchar* data = PyArray_BYTES(arr_);
	auto* allocator =
	NpyString_acquire_allocator(reinterpret_cast<PyArray_StringDTypeObject*>(dtype_));
	std::unique_ptr<npy_string_allocator, decltype(&NpyString_release_allocator)>
	allocator_guard(allocator, &NpyString_release_allocator);
	constbool has_mask = mask_ != nullptr;
	Ndarray1DIndexer<uint8_t> mask_values;
	if (has_mask) {
	mask_values = Ndarray1DIndexer<uint8_t>(mask_);
	}
	constexprint64_tkBatchSize = 4096;
	// NpyString_load returns borrowed views that remain valid while the allocator is
	// locked.
	constint64_t batch_capacity = std::min(length_, kBatchSize);
	std::vector<std::string_view> values(batch_capacity);
	std::vector<uint8_t> valid(batch_capacity);
	npy_static_string value{};
	for (int64_t offset = 0; offset < length_; offset += kBatchSize) {
	constchar* data = PyArray_BYTES(arr_);
	constbool has_mask = mask_ != nullptr;
	Ndarray1DIndexer<uint8_t> mask_values;
	if (has_mask) {
	mask_values = Ndarray1DIndexer<uint8_t>(mask_);
	}
	constexprint64_tkBatchSize = 4096;
	// NpyString_load returns borrowed views that remain valid while the allocator is
	// locked.
	constint64_t batch_capacity = std::min(length_, kBatchSize);
	std::vector<std::string_view> values(batch_capacity);
	std::vector<uint8_t> valid(batch_capacity);
	npy_static_string value{};
	// Hold the allocator lock for the whole conversion to ensure a consistent snapshot
	auto* allocator =
	NpyString_acquire_allocator(reinterpret_cast<PyArray_StringDTypeObject*>(dtype_));
	std::unique_ptr<npy_string_allocator, decltype(&NpyString_release_allocator)>
	allocator_guard(allocator, &NpyString_release_allocator);
	for (int64_t offset = 0; offset < length_; offset += kBatchSize) {

This way you hold the allocator lock for the minimum amount of code, I also added a comment explaining that there is a mutex here so a reader knows to watch out for deadlock risks.

[ngoldbaum]

Suggested change

	return VisitStringDType(builder);
	// Acquires a lock so this must not be moved inside the gil_lock section below
	return VisitStringDType(builder);

[ngoldbaum]

Might be nice to see the type that was passed in:

Suggested change

	"NumPy StringDType can only be converted to Arrow string types");
	"NumPy StringDType can only be converted to Arrow string types, got",
	type_->ToString());

[ngoldbaum]

My AI model thinks that doing this has an unintended, untested side effect: pa.array([np.array([...], dtype="T")]) now infers as a list of strings and converts through the per-element sequence fallback in python_to_arrow.cc:

arrow/python/pyarrow/src/arrow/python/python_to_arrow.cc

Lines 945 to 947 in 54ea6d5

	if (PyArray_DESCR(ndarray)->type_num != NUMPY_TYPE) { \
	returnthis->value_converter_->Extend(value, size); \
	} \

That's fine, it just needs a test to cover it.

[ngoldbaum]

This is a public header so this shouldn't be deleted. Deleting this also caused churn in this PR in other compilation units, which newly add #include <numeric> to work around this. The same could happen in user code including this header.

[ngoldbaum]

Suggested change

	@pytest.mark.parametrize('string_type', [pa.large_string(), pa.string_view()])
	@pytest.mark.parametrize('string_type', [None, pa.large_string(), pa.string_view()])

Let's include the default string() type too.

[ngoldbaum]

Why is the output missing sentinel None for all input na_object choices, in particular for strings?

Note that this also disagrees with NumPy:

>>> np.array(["hello", "__placeholder__", "world"], dtype="T").tolist()
['hello', '__placeholder__', 'world']

Is there a reason why you can't more faithfully translate NumPy's string missing data semantics?

[ngoldbaum]

The first test improves coverage for your changes to ChunkedBinaryBuilder. The second test covers the untested case I referred to in my comment above where a list of ndarrays gains new inference behavior.

Suggested change

	@pytest.mark.numpy
	deftest_array_from_numpy_string_dtype_chunking(numpy_string_dtype):
	# Three 6 MiB values in one batch must split across the 16 MiB
	# per-chunk limit of the string() path.
	values= ["x"* (6*1024*1024)] *3
	arr=np.array(values, dtype=numpy_string_dtype())
	result=pa.array(arr, type=pa.string())
	assertisinstance(result, pa.ChunkedArray)
	assertresult.num_chunks==2
	result.validate(full=True)
	assertresult.to_pylist() ==values
	@pytest.mark.numpy
	deftest_array_from_list_of_numpy_string_dtype_arrays(numpy_string_dtype):
	values= [["a", "bb"], ["ccc"]]
	arrays= [np.array(v, dtype=numpy_string_dtype()) forvinvalues]
	result=pa.array(arrays)
	assertresult.type==pa.list_(pa.string())
	assertresult.to_pylist() ==values

[jorisvandenbossche]

@alippai your previous PR #48391 did not need changes to the Builder code, while now there seems to be a significant addition there. Can you provide some context regarding the different implementation strategy?

[jorisvandenbossche]

Inline comment of @ngoldbaum (moving it here it make it more prominent and not hidden in a potentially outdated/collapsed review comment):

Why is the output missing sentinel None for all input na_object choices, in particular for strings?

Note that this also disagrees with NumPy:

>>> np.array(["hello", "__placeholder__", "world"], dtype="T").tolist()
['hello', '__placeholder__', 'world']

Is there a reason why you can't more faithfully translate NumPy's string missing data semantics?

I think this is the main API design question to discuss: the numpy StringDtype has a configurable missing value sentinel model, while Arrow has no such option and just has nulls (through the bitmask).
So that means it is not really possible to fully "faithfully" translate those missing semantics in the numpy->arrow conversion. Either:

• we use the actual placeholder value (or at least if it is a string) as the resulting string value in the Arrow string array. That preserves the value, but looses the fact that it is missing
• we translate the placeholder values to nulls (what this PR currently does). That preserves the missingness, but looses the information about the original placeholder value used.

So we always loose something (unless we would create an extension type ..). Personally, my feeling is that preserving "missingness" is the most relevant.

FWIW, this also means that a fully faithful roundtrip (once the conversion from arrow -> numpy exists) is also not possible out of the box, only if the user specifies the resulting numpy dtype (which has the information about which placeholder to use)

[alippai]

@alippai your previous PR #48391 did not need changes to the Builder code, while now there seems to be a significant addition there. Can you provide some context regarding the different implementation strategy?

@jorisvandenbossche this PR has a similar simpler version as the first commit. I opportunistically added a few batching, performance optimizations in the following 2-3rd commits (without benchmarks, I don’t know how trivial is this considered and I don’t have a representative machine for this right now).

[ngoldbaum]

this PR has a similar simpler version as the first commit

Maybe as a first pass you could try to just merge the simpler version. Then later in future PRs you could add the optimizations, along with benchmarks to justify them.

[ngoldbaum]

@alippai gentle ping here. Just in case it's helpful: I'm happy to take over shepherding this feature as I can work on it under funded time.

[alippai]

@ngoldbaum I can work on this later this weekend only. Feel free to either take the first commit or start from scratch

[ngoldbaum]

I went ahead and opened #51157 which I marked as superseding this one.

[alippai]

Rationale for this change

Implement #42018

What changes are included in this PR?

Conversion in numpy->arrow direction with multiple string types

Are these changes tested?

Two basic conversion tests added and one boundary that only string(), large_string(), string_view() is supported.

Are there any user-facing changes?

Yes, adds support to numpy.StringDType as source

cc @jorisvandenbossche as he opened the original issue

This PR was created using GPT-5.6-Sol-xhigh, every line read & reviewed by me.

• GitHub Issue: [Python] Conversion to/from numpy 2.0+ new StringDType #42018

[alippai]

@ngoldbaum can I ask you for review?

[alippai]

The commits are in complexity & speed order. They are supposed to be reviewable commit-by-commit.

[ngoldbaum]

I'll do a pass over this next week. Thank you for moving this forward.

[ngoldbaum]

The two CI failures are unrelated, I think.

Before this can be merged, you should also explicitly mention this feature in docs/source/python/numpy.rst. Right now it just says arrow supports "structured dtypes or strings", but NumPy's string support has grown more complicated since that was written and it's worth updating now to say something like "structured dtypes and both fixed-width, and variable-width strings". Maybe do a further pass to see if there are other spots that could mention this.

I carefully reviewed this for lock discipline and didn't spot any bugs. I do not see any places where possibly-blocking APIs are called while the allocator lock is held. Also the way this is currently structured, it can't conflict with the GIL because the conversion happens in code that explicitly does not hold the GIL.

I have some minor comments below but I think this is in mostly good shape from the perspective of NumPy C API use.

[ngoldbaum]

Suggested change

	constchar* data = PyArray_BYTES(arr_);
	auto* allocator =
	NpyString_acquire_allocator(reinterpret_cast<PyArray_StringDTypeObject*>(dtype_));
	std::unique_ptr<npy_string_allocator, decltype(&NpyString_release_allocator)>
	allocator_guard(allocator, &NpyString_release_allocator);
	constbool has_mask = mask_ != nullptr;
	Ndarray1DIndexer<uint8_t> mask_values;
	if (has_mask) {
	mask_values = Ndarray1DIndexer<uint8_t>(mask_);
	}
	constexprint64_tkBatchSize = 4096;
	// NpyString_load returns borrowed views that remain valid while the allocator is
	// locked.
	constint64_t batch_capacity = std::min(length_, kBatchSize);
	std::vector<std::string_view> values(batch_capacity);
	std::vector<uint8_t> valid(batch_capacity);
	npy_static_string value{};
	for (int64_t offset = 0; offset < length_; offset += kBatchSize) {
	constchar* data = PyArray_BYTES(arr_);
	constbool has_mask = mask_ != nullptr;
	Ndarray1DIndexer<uint8_t> mask_values;
	if (has_mask) {
	mask_values = Ndarray1DIndexer<uint8_t>(mask_);
	}
	constexprint64_tkBatchSize = 4096;
	// NpyString_load returns borrowed views that remain valid while the allocator is
	// locked.
	constint64_t batch_capacity = std::min(length_, kBatchSize);
	std::vector<std::string_view> values(batch_capacity);
	std::vector<uint8_t> valid(batch_capacity);
	npy_static_string value{};
	// Hold the allocator lock for the whole conversion to ensure a consistent snapshot
	auto* allocator =
	NpyString_acquire_allocator(reinterpret_cast<PyArray_StringDTypeObject*>(dtype_));
	std::unique_ptr<npy_string_allocator, decltype(&NpyString_release_allocator)>
	allocator_guard(allocator, &NpyString_release_allocator);
	for (int64_t offset = 0; offset < length_; offset += kBatchSize) {

This way you hold the allocator lock for the minimum amount of code, I also added a comment explaining that there is a mutex here so a reader knows to watch out for deadlock risks.

[ngoldbaum]

Suggested change

	return VisitStringDType(builder);
	// Acquires a lock so this must not be moved inside the gil_lock section below
	return VisitStringDType(builder);

[ngoldbaum]

Might be nice to see the type that was passed in:

Suggested change

	"NumPy StringDType can only be converted to Arrow string types");
	"NumPy StringDType can only be converted to Arrow string types, got",
	type_->ToString());

[ngoldbaum]

My AI model thinks that doing this has an unintended, untested side effect: pa.array([np.array([...], dtype="T")]) now infers as a list of strings and converts through the per-element sequence fallback in python_to_arrow.cc:

arrow/python/pyarrow/src/arrow/python/python_to_arrow.cc

Lines 945 to 947 in 54ea6d5

	if (PyArray_DESCR(ndarray)->type_num != NUMPY_TYPE) { \
	returnthis->value_converter_->Extend(value, size); \
	} \

That's fine, it just needs a test to cover it.

[ngoldbaum]

This is a public header so this shouldn't be deleted. Deleting this also caused churn in this PR in other compilation units, which newly add #include <numeric> to work around this. The same could happen in user code including this header.

[ngoldbaum]

Suggested change

	@pytest.mark.parametrize('string_type', [pa.large_string(), pa.string_view()])
	@pytest.mark.parametrize('string_type', [None, pa.large_string(), pa.string_view()])

Let's include the default string() type too.

[ngoldbaum]

Why is the output missing sentinel None for all input na_object choices, in particular for strings?

Note that this also disagrees with NumPy:

>>> np.array(["hello", "__placeholder__", "world"], dtype="T").tolist()
['hello', '__placeholder__', 'world']

Is there a reason why you can't more faithfully translate NumPy's string missing data semantics?

[ngoldbaum]

The first test improves coverage for your changes to ChunkedBinaryBuilder. The second test covers the untested case I referred to in my comment above where a list of ndarrays gains new inference behavior.

Suggested change

	@pytest.mark.numpy
	deftest_array_from_numpy_string_dtype_chunking(numpy_string_dtype):
	# Three 6 MiB values in one batch must split across the 16 MiB
	# per-chunk limit of the string() path.
	values= ["x"* (6*1024*1024)] *3
	arr=np.array(values, dtype=numpy_string_dtype())
	result=pa.array(arr, type=pa.string())
	assertisinstance(result, pa.ChunkedArray)
	assertresult.num_chunks==2
	result.validate(full=True)
	assertresult.to_pylist() ==values
	@pytest.mark.numpy
	deftest_array_from_list_of_numpy_string_dtype_arrays(numpy_string_dtype):
	values= [["a", "bb"], ["ccc"]]
	arrays= [np.array(v, dtype=numpy_string_dtype()) forvinvalues]
	result=pa.array(arrays)
	assertresult.type==pa.list_(pa.string())
	assertresult.to_pylist() ==values

[jorisvandenbossche]

@alippai your previous PR #48391 did not need changes to the Builder code, while now there seems to be a significant addition there. Can you provide some context regarding the different implementation strategy?

[jorisvandenbossche]

Inline comment of @ngoldbaum (moving it here it make it more prominent and not hidden in a potentially outdated/collapsed review comment):

Why is the output missing sentinel None for all input na_object choices, in particular for strings?

Note that this also disagrees with NumPy:

>>> np.array(["hello", "__placeholder__", "world"], dtype="T").tolist()
['hello', '__placeholder__', 'world']

Is there a reason why you can't more faithfully translate NumPy's string missing data semantics?

I think this is the main API design question to discuss: the numpy StringDtype has a configurable missing value sentinel model, while Arrow has no such option and just has nulls (through the bitmask).
So that means it is not really possible to fully "faithfully" translate those missing semantics in the numpy->arrow conversion. Either:

• we use the actual placeholder value (or at least if it is a string) as the resulting string value in the Arrow string array. That preserves the value, but looses the fact that it is missing
• we translate the placeholder values to nulls (what this PR currently does). That preserves the missingness, but looses the information about the original placeholder value used.

So we always loose something (unless we would create an extension type ..). Personally, my feeling is that preserving "missingness" is the most relevant.

FWIW, this also means that a fully faithful roundtrip (once the conversion from arrow -> numpy exists) is also not possible out of the box, only if the user specifies the resulting numpy dtype (which has the information about which placeholder to use)

[alippai]

@alippai your previous PR #48391 did not need changes to the Builder code, while now there seems to be a significant addition there. Can you provide some context regarding the different implementation strategy?

@jorisvandenbossche this PR has a similar simpler version as the first commit. I opportunistically added a few batching, performance optimizations in the following 2-3rd commits (without benchmarks, I don’t know how trivial is this considered and I don’t have a representative machine for this right now).

[ngoldbaum]

this PR has a similar simpler version as the first commit

Maybe as a first pass you could try to just merge the simpler version. Then later in future PRs you could add the optimizations, along with benchmarks to justify them.

[ngoldbaum]

@alippai gentle ping here. Just in case it's helpful: I'm happy to take over shepherding this feature as I can work on it under funded time.

[alippai]

@ngoldbaum I can work on this later this weekend only. Feel free to either take the first commit or start from scratch

[ngoldbaum]

I went ahead and opened #51157 which I marked as superseding this one.

[alippai]

Rationale for this change

Implement #42018

What changes are included in this PR?

Conversion in numpy->arrow direction with multiple string types

Are these changes tested?

Two basic conversion tests added and one boundary that only string(), large_string(), string_view() is supported.

Are there any user-facing changes?

Yes, adds support to numpy.StringDType as source

cc @jorisvandenbossche as he opened the original issue

This PR was created using GPT-5.6-Sol-xhigh, every line read & reviewed by me.

• GitHub Issue: [Python] Conversion to/from numpy 2.0+ new StringDType #42018

[alippai]

@ngoldbaum can I ask you for review?

[alippai]

The commits are in complexity & speed order. They are supposed to be reviewable commit-by-commit.

[ngoldbaum]

I'll do a pass over this next week. Thank you for moving this forward.

[ngoldbaum]

The two CI failures are unrelated, I think.

Before this can be merged, you should also explicitly mention this feature in docs/source/python/numpy.rst. Right now it just says arrow supports "structured dtypes or strings", but NumPy's string support has grown more complicated since that was written and it's worth updating now to say something like "structured dtypes and both fixed-width, and variable-width strings". Maybe do a further pass to see if there are other spots that could mention this.

I carefully reviewed this for lock discipline and didn't spot any bugs. I do not see any places where possibly-blocking APIs are called while the allocator lock is held. Also the way this is currently structured, it can't conflict with the GIL because the conversion happens in code that explicitly does not hold the GIL.

I have some minor comments below but I think this is in mostly good shape from the perspective of NumPy C API use.

[ngoldbaum]

Suggested change

	constchar* data = PyArray_BYTES(arr_);
	auto* allocator =
	NpyString_acquire_allocator(reinterpret_cast<PyArray_StringDTypeObject*>(dtype_));
	std::unique_ptr<npy_string_allocator, decltype(&NpyString_release_allocator)>
	allocator_guard(allocator, &NpyString_release_allocator);
	constbool has_mask = mask_ != nullptr;
	Ndarray1DIndexer<uint8_t> mask_values;
	if (has_mask) {
	mask_values = Ndarray1DIndexer<uint8_t>(mask_);
	}
	constexprint64_tkBatchSize = 4096;
	// NpyString_load returns borrowed views that remain valid while the allocator is
	// locked.
	constint64_t batch_capacity = std::min(length_, kBatchSize);
	std::vector<std::string_view> values(batch_capacity);
	std::vector<uint8_t> valid(batch_capacity);
	npy_static_string value{};
	for (int64_t offset = 0; offset < length_; offset += kBatchSize) {
	constchar* data = PyArray_BYTES(arr_);
	constbool has_mask = mask_ != nullptr;
	Ndarray1DIndexer<uint8_t> mask_values;
	if (has_mask) {
	mask_values = Ndarray1DIndexer<uint8_t>(mask_);
	}
	constexprint64_tkBatchSize = 4096;
	// NpyString_load returns borrowed views that remain valid while the allocator is
	// locked.
	constint64_t batch_capacity = std::min(length_, kBatchSize);
	std::vector<std::string_view> values(batch_capacity);
	std::vector<uint8_t> valid(batch_capacity);
	npy_static_string value{};
	// Hold the allocator lock for the whole conversion to ensure a consistent snapshot
	auto* allocator =
	NpyString_acquire_allocator(reinterpret_cast<PyArray_StringDTypeObject*>(dtype_));
	std::unique_ptr<npy_string_allocator, decltype(&NpyString_release_allocator)>
	allocator_guard(allocator, &NpyString_release_allocator);
	for (int64_t offset = 0; offset < length_; offset += kBatchSize) {

This way you hold the allocator lock for the minimum amount of code, I also added a comment explaining that there is a mutex here so a reader knows to watch out for deadlock risks.

[ngoldbaum]

Suggested change

	return VisitStringDType(builder);
	// Acquires a lock so this must not be moved inside the gil_lock section below
	return VisitStringDType(builder);

[ngoldbaum]

Might be nice to see the type that was passed in:

Suggested change

	"NumPy StringDType can only be converted to Arrow string types");
	"NumPy StringDType can only be converted to Arrow string types, got",
	type_->ToString());

[ngoldbaum]

My AI model thinks that doing this has an unintended, untested side effect: pa.array([np.array([...], dtype="T")]) now infers as a list of strings and converts through the per-element sequence fallback in python_to_arrow.cc:

arrow/python/pyarrow/src/arrow/python/python_to_arrow.cc

Lines 945 to 947 in 54ea6d5

	if (PyArray_DESCR(ndarray)->type_num != NUMPY_TYPE) { \
	returnthis->value_converter_->Extend(value, size); \
	} \

That's fine, it just needs a test to cover it.

[ngoldbaum]

This is a public header so this shouldn't be deleted. Deleting this also caused churn in this PR in other compilation units, which newly add #include <numeric> to work around this. The same could happen in user code including this header.

[ngoldbaum]

Suggested change

	@pytest.mark.parametrize('string_type', [pa.large_string(), pa.string_view()])
	@pytest.mark.parametrize('string_type', [None, pa.large_string(), pa.string_view()])

Let's include the default string() type too.

[ngoldbaum]

Why is the output missing sentinel None for all input na_object choices, in particular for strings?

Note that this also disagrees with NumPy:

>>> np.array(["hello", "__placeholder__", "world"], dtype="T").tolist()
['hello', '__placeholder__', 'world']

Is there a reason why you can't more faithfully translate NumPy's string missing data semantics?

[ngoldbaum]

The first test improves coverage for your changes to ChunkedBinaryBuilder. The second test covers the untested case I referred to in my comment above where a list of ndarrays gains new inference behavior.

Suggested change

	@pytest.mark.numpy
	deftest_array_from_numpy_string_dtype_chunking(numpy_string_dtype):
	# Three 6 MiB values in one batch must split across the 16 MiB
	# per-chunk limit of the string() path.
	values= ["x"* (6*1024*1024)] *3
	arr=np.array(values, dtype=numpy_string_dtype())
	result=pa.array(arr, type=pa.string())
	assertisinstance(result, pa.ChunkedArray)
	assertresult.num_chunks==2
	result.validate(full=True)
	assertresult.to_pylist() ==values
	@pytest.mark.numpy
	deftest_array_from_list_of_numpy_string_dtype_arrays(numpy_string_dtype):
	values= [["a", "bb"], ["ccc"]]
	arrays= [np.array(v, dtype=numpy_string_dtype()) forvinvalues]
	result=pa.array(arrays)
	assertresult.type==pa.list_(pa.string())
	assertresult.to_pylist() ==values

[jorisvandenbossche]

@alippai your previous PR #48391 did not need changes to the Builder code, while now there seems to be a significant addition there. Can you provide some context regarding the different implementation strategy?

[jorisvandenbossche]

Inline comment of @ngoldbaum (moving it here it make it more prominent and not hidden in a potentially outdated/collapsed review comment):

Why is the output missing sentinel None for all input na_object choices, in particular for strings?

Note that this also disagrees with NumPy:

>>> np.array(["hello", "__placeholder__", "world"], dtype="T").tolist()
['hello', '__placeholder__', 'world']

Is there a reason why you can't more faithfully translate NumPy's string missing data semantics?

I think this is the main API design question to discuss: the numpy StringDtype has a configurable missing value sentinel model, while Arrow has no such option and just has nulls (through the bitmask).
So that means it is not really possible to fully "faithfully" translate those missing semantics in the numpy->arrow conversion. Either:

• we use the actual placeholder value (or at least if it is a string) as the resulting string value in the Arrow string array. That preserves the value, but looses the fact that it is missing
• we translate the placeholder values to nulls (what this PR currently does). That preserves the missingness, but looses the information about the original placeholder value used.

So we always loose something (unless we would create an extension type ..). Personally, my feeling is that preserving "missingness" is the most relevant.

FWIW, this also means that a fully faithful roundtrip (once the conversion from arrow -> numpy exists) is also not possible out of the box, only if the user specifies the resulting numpy dtype (which has the information about which placeholder to use)

[alippai]

@alippai your previous PR #48391 did not need changes to the Builder code, while now there seems to be a significant addition there. Can you provide some context regarding the different implementation strategy?

@jorisvandenbossche this PR has a similar simpler version as the first commit. I opportunistically added a few batching, performance optimizations in the following 2-3rd commits (without benchmarks, I don’t know how trivial is this considered and I don’t have a representative machine for this right now).

[ngoldbaum]

this PR has a similar simpler version as the first commit

Maybe as a first pass you could try to just merge the simpler version. Then later in future PRs you could add the optimizations, along with benchmarks to justify them.

[ngoldbaum]

@alippai gentle ping here. Just in case it's helpful: I'm happy to take over shepherding this feature as I can work on it under funded time.

[alippai]

@ngoldbaum I can work on this later this weekend only. Feel free to either take the first commit or start from scratch

[ngoldbaum]

I went ahead and opened #51157 which I marked as superseding this one.

[alippai]

Rationale for this change

Implement #42018

What changes are included in this PR?

Conversion in numpy->arrow direction with multiple string types

Are these changes tested?

Two basic conversion tests added and one boundary that only string(), large_string(), string_view() is supported.

Are there any user-facing changes?

Yes, adds support to numpy.StringDType as source

cc @jorisvandenbossche as he opened the original issue

This PR was created using GPT-5.6-Sol-xhigh, every line read & reviewed by me.

• GitHub Issue: [Python] Conversion to/from numpy 2.0+ new StringDType #42018

[alippai]

@ngoldbaum can I ask you for review?

[alippai]

The commits are in complexity & speed order. They are supposed to be reviewable commit-by-commit.

[ngoldbaum]

I'll do a pass over this next week. Thank you for moving this forward.

[ngoldbaum]

The two CI failures are unrelated, I think.

Before this can be merged, you should also explicitly mention this feature in docs/source/python/numpy.rst. Right now it just says arrow supports "structured dtypes or strings", but NumPy's string support has grown more complicated since that was written and it's worth updating now to say something like "structured dtypes and both fixed-width, and variable-width strings". Maybe do a further pass to see if there are other spots that could mention this.

I carefully reviewed this for lock discipline and didn't spot any bugs. I do not see any places where possibly-blocking APIs are called while the allocator lock is held. Also the way this is currently structured, it can't conflict with the GIL because the conversion happens in code that explicitly does not hold the GIL.

I have some minor comments below but I think this is in mostly good shape from the perspective of NumPy C API use.

[ngoldbaum]

Suggested change

	constchar* data = PyArray_BYTES(arr_);
	auto* allocator =
	NpyString_acquire_allocator(reinterpret_cast<PyArray_StringDTypeObject*>(dtype_));
	std::unique_ptr<npy_string_allocator, decltype(&NpyString_release_allocator)>
	allocator_guard(allocator, &NpyString_release_allocator);
	constbool has_mask = mask_ != nullptr;
	Ndarray1DIndexer<uint8_t> mask_values;
	if (has_mask) {
	mask_values = Ndarray1DIndexer<uint8_t>(mask_);
	}
	constexprint64_tkBatchSize = 4096;
	// NpyString_load returns borrowed views that remain valid while the allocator is
	// locked.
	constint64_t batch_capacity = std::min(length_, kBatchSize);
	std::vector<std::string_view> values(batch_capacity);
	std::vector<uint8_t> valid(batch_capacity);
	npy_static_string value{};
	for (int64_t offset = 0; offset < length_; offset += kBatchSize) {
	constchar* data = PyArray_BYTES(arr_);
	constbool has_mask = mask_ != nullptr;
	Ndarray1DIndexer<uint8_t> mask_values;
	if (has_mask) {
	mask_values = Ndarray1DIndexer<uint8_t>(mask_);
	}
	constexprint64_tkBatchSize = 4096;
	// NpyString_load returns borrowed views that remain valid while the allocator is
	// locked.
	constint64_t batch_capacity = std::min(length_, kBatchSize);
	std::vector<std::string_view> values(batch_capacity);
	std::vector<uint8_t> valid(batch_capacity);
	npy_static_string value{};
	// Hold the allocator lock for the whole conversion to ensure a consistent snapshot
	auto* allocator =
	NpyString_acquire_allocator(reinterpret_cast<PyArray_StringDTypeObject*>(dtype_));
	std::unique_ptr<npy_string_allocator, decltype(&NpyString_release_allocator)>
	allocator_guard(allocator, &NpyString_release_allocator);
	for (int64_t offset = 0; offset < length_; offset += kBatchSize) {

This way you hold the allocator lock for the minimum amount of code, I also added a comment explaining that there is a mutex here so a reader knows to watch out for deadlock risks.

[ngoldbaum]

Suggested change

	return VisitStringDType(builder);
	// Acquires a lock so this must not be moved inside the gil_lock section below
	return VisitStringDType(builder);

[ngoldbaum]

Might be nice to see the type that was passed in:

Suggested change

	"NumPy StringDType can only be converted to Arrow string types");
	"NumPy StringDType can only be converted to Arrow string types, got",
	type_->ToString());

[ngoldbaum]

My AI model thinks that doing this has an unintended, untested side effect: pa.array([np.array([...], dtype="T")]) now infers as a list of strings and converts through the per-element sequence fallback in python_to_arrow.cc:

arrow/python/pyarrow/src/arrow/python/python_to_arrow.cc

Lines 945 to 947 in 54ea6d5

	if (PyArray_DESCR(ndarray)->type_num != NUMPY_TYPE) { \
	returnthis->value_converter_->Extend(value, size); \
	} \

That's fine, it just needs a test to cover it.

[ngoldbaum]

This is a public header so this shouldn't be deleted. Deleting this also caused churn in this PR in other compilation units, which newly add #include <numeric> to work around this. The same could happen in user code including this header.

[ngoldbaum]

Suggested change

	@pytest.mark.parametrize('string_type', [pa.large_string(), pa.string_view()])
	@pytest.mark.parametrize('string_type', [None, pa.large_string(), pa.string_view()])

Let's include the default string() type too.

[ngoldbaum]

Why is the output missing sentinel None for all input na_object choices, in particular for strings?

Note that this also disagrees with NumPy:

>>> np.array(["hello", "__placeholder__", "world"], dtype="T").tolist()
['hello', '__placeholder__', 'world']

Is there a reason why you can't more faithfully translate NumPy's string missing data semantics?

[ngoldbaum]

The first test improves coverage for your changes to ChunkedBinaryBuilder. The second test covers the untested case I referred to in my comment above where a list of ndarrays gains new inference behavior.

Suggested change

	@pytest.mark.numpy
	deftest_array_from_numpy_string_dtype_chunking(numpy_string_dtype):
	# Three 6 MiB values in one batch must split across the 16 MiB
	# per-chunk limit of the string() path.
	values= ["x"* (6*1024*1024)] *3
	arr=np.array(values, dtype=numpy_string_dtype())
	result=pa.array(arr, type=pa.string())
	assertisinstance(result, pa.ChunkedArray)
	assertresult.num_chunks==2
	result.validate(full=True)
	assertresult.to_pylist() ==values
	@pytest.mark.numpy
	deftest_array_from_list_of_numpy_string_dtype_arrays(numpy_string_dtype):
	values= [["a", "bb"], ["ccc"]]
	arrays= [np.array(v, dtype=numpy_string_dtype()) forvinvalues]
	result=pa.array(arrays)
	assertresult.type==pa.list_(pa.string())
	assertresult.to_pylist() ==values

[jorisvandenbossche]

@alippai your previous PR #48391 did not need changes to the Builder code, while now there seems to be a significant addition there. Can you provide some context regarding the different implementation strategy?

[jorisvandenbossche]

Inline comment of @ngoldbaum (moving it here it make it more prominent and not hidden in a potentially outdated/collapsed review comment):

Why is the output missing sentinel None for all input na_object choices, in particular for strings?

Note that this also disagrees with NumPy:

>>> np.array(["hello", "__placeholder__", "world"], dtype="T").tolist()
['hello', '__placeholder__', 'world']

Is there a reason why you can't more faithfully translate NumPy's string missing data semantics?

I think this is the main API design question to discuss: the numpy StringDtype has a configurable missing value sentinel model, while Arrow has no such option and just has nulls (through the bitmask).
So that means it is not really possible to fully "faithfully" translate those missing semantics in the numpy->arrow conversion. Either:

• we use the actual placeholder value (or at least if it is a string) as the resulting string value in the Arrow string array. That preserves the value, but looses the fact that it is missing
• we translate the placeholder values to nulls (what this PR currently does). That preserves the missingness, but looses the information about the original placeholder value used.

So we always loose something (unless we would create an extension type ..). Personally, my feeling is that preserving "missingness" is the most relevant.

FWIW, this also means that a fully faithful roundtrip (once the conversion from arrow -> numpy exists) is also not possible out of the box, only if the user specifies the resulting numpy dtype (which has the information about which placeholder to use)

[alippai]

@alippai your previous PR #48391 did not need changes to the Builder code, while now there seems to be a significant addition there. Can you provide some context regarding the different implementation strategy?

@jorisvandenbossche this PR has a similar simpler version as the first commit. I opportunistically added a few batching, performance optimizations in the following 2-3rd commits (without benchmarks, I don’t know how trivial is this considered and I don’t have a representative machine for this right now).

[ngoldbaum]

this PR has a similar simpler version as the first commit

Maybe as a first pass you could try to just merge the simpler version. Then later in future PRs you could add the optimizations, along with benchmarks to justify them.

[ngoldbaum]

@alippai gentle ping here. Just in case it's helpful: I'm happy to take over shepherding this feature as I can work on it under funded time.

[alippai]

@ngoldbaum I can work on this later this weekend only. Feel free to either take the first commit or start from scratch

[ngoldbaum]

I went ahead and opened #51157 which I marked as superseding this one.