GH-35289: [Python] Support large variable width types in numpy conversion - #36701

⚠️ GitHub issue #35289 has been automatically assigned in GitHub to PR creator.

Thank you for the contribution @Kimahriman !

The PR looks good!
As you have mentioned, there might be some improvement possible with consolidating the common code.

In the C++ part, where I am a novice also - but, I would try with creating a helper Status function and see if I can make it work with both builder instances or not (am not really sure it will work).

As for the python test, it could also be consolidated with test_numpy_binary_overflow_to_chunked with parametrizing the test function as for example in

If there's a way to consolidate the common code let me know, this is my first C++ in a long time.

In the Visit functions, I think the main (only?) difference is the type of builder that was created? In that case I think it should be possible to template this: have a single VisitString() that is templated on the builder type, and then the Visit(const StringType& type) could be a small wrapper around calling VisitString<ChunkedStringBuilder>(). Although the builder having different parameters to instantiate it might complicate things ..

Thanks for the ideas, I'll see if I can figure anything out 😅

Ok I learned about C++ templates and deduped the different functions, and I just parametrized the existing python test

@jorisvandenbossche gentle ping

@jorisvandenbossche gentle ping again. Merged in master and resolved the conflict

Looks good!

And apologies for the very slow follow-up ..

I have one more request: could you add a simple test for this as well (the large_memory test you edited it good to have, but in most our CI builds those are skipped, so it would be good to have a simple small test as well that is run everywhere). For example there is a test_array_from_numpy_ascii and test_array_from_numpy_unicode. You could add one case to those tests with specifying the type as the large variant.

Parameterized those tests to specify the type and use regular and large types

I can review or pickup next week. @Kimahriman are you interested in continuing this work?

I can review or pickup next week. @Kimahriman are you interested in continuing this work?

I was able to get this up to date with main, I don't know much about the view types

Thank you @Kimahriman! Depending on how adventurous you feel we can make a separate PR or add view types here.

Probably would be good to do in a follow on, not sure how quickly I can figure out and get the views working

@Kimahriman agreed!

Nevermind adding the views was pretty simple

⚠️ GitHub issue #35289 has been automatically assigned in GitHub to PR creator.

⚠️ GitHub issue #35289 has been automatically assigned in GitHub to PR creator.

All the open discussions should be resolved, and I updated the description a bit with the current implementation

@github-actions crossbow submit -g python

Revision: cb479c8

Submitted crossbow builds: ursacomputing/crossbow @ actions-4fe3452fe3

Task	Status
example-python-minimal-build-fedora-conda
example-python-minimal-build-ubuntu-venv
test-conda-python-3.10
test-conda-python-3.10-hdfs-2.9.2
test-conda-python-3.10-hdfs-3.2.1
test-conda-python-3.10-pandas-latest-numpy-latest
test-conda-python-3.11
test-conda-python-3.11-dask-latest
test-conda-python-3.11-dask-upstream_devel
test-conda-python-3.11-hypothesis
test-conda-python-3.11-pandas-latest-numpy-1.26
test-conda-python-3.11-pandas-latest-numpy-latest
test-conda-python-3.11-pandas-nightly-numpy-nightly
test-conda-python-3.11-pandas-upstream_devel-numpy-nightly
test-conda-python-3.11-spark-master
test-conda-python-3.12
test-conda-python-3.12-cpython-debug
test-conda-python-3.13
test-conda-python-3.9
test-conda-python-3.9-pandas-1.1.3-numpy-1.19.5
test-conda-python-emscripten
test-cuda-python-ubuntu-22.04-cuda-11.7.1
test-debian-12-python-3-amd64
test-debian-12-python-3-i386
test-fedora-39-python-3
test-ubuntu-22.04-python-3
test-ubuntu-22.04-python-313-freethreading
test-ubuntu-24.04-python-3

All failing builds are known, test-conda-python-3.11-hypothesis has an issue opened: #45292 and two minimal builds also: #44803. None of it is connected to this PR. @rok want to have a look before I merge?

Thank you for your contribution and patience @Kimahriman !

After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 9752c34.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 13 possible false positives for unstable benchmarks that are known to sometimes produce them.