GH-50312: [Python] Fix UUID extension type round-trip to pandas returning bytes#50325
GH-50312: [Python] Fix UUID extension type round-trip to pandas returning bytes#50325parker-cassar wants to merge 1 commit into
Conversation
|
|
5b15a46 to
386b1b6
Compare
|
Hey @parker-cassar, thanks so much for jumping on this so quickly. Just out of curiosity—since this delegates to to_pylist(), do you think this pattern might end up being useful for other Arrow extension types down the way??? |
rok
left a comment
There was a problem hiding this comment.
Thanks for working on this @parker-cassar!
A couple of questions since we're aiming to support Pandas 3.x and Pandas 2.x too for a while.
| assert result_df.loc[2, "id"] == u2 | ||
| assert pd.isna(result_df.loc[1, "id"]) | ||
|
|
||
|
|
There was a problem hiding this comment.
Would this test pass with the current proposal?
| @pytest.mark.pandas | |
| def test_uuid_array_to_pandas(): | |
| from uuid import uuid4 | |
| import pandas as pd | |
| import pandas.testing as tm | |
| values = [uuid4(), None, uuid4()] | |
| arr = pa.array(values, type=pa.uuid()) | |
| result = arr.to_pandas() | |
| expected = pd.Series(values, dtype=object) | |
| tm.assert_series_equal(result, expected) |
There was a problem hiding this comment.
No, this did not pass with the 2-D return. Applied your approach, from_arrow returns 1-D now and the reshape moved to _reconstruct_block. test passes locally.
Note: I squashed my 4 iterative commits into one during a rebase (3rd commit was just resyncing main). Then amended that squashed commit with a small pep 8 fix. The changes since your review are your two suggestions. Sorry for the noise, will avoid the mid-review force-pushes going forward.
ea0d9f2 to
b7f87d1
Compare
Maybe! It works here because the target is object dtype anyway, so building the Python objects is unavoidable. Other extension types could use the same from_arrow + reshape structure, but anything performance-sensitive would want a zero-copy path instead of going through Python objects. |
5d2cf7e to
bc83f11
Compare
bc83f11 to
457ff20
Compare
|
@rok Applied both of your suggestions. Ready for another look whenever you have time. |
|
@parker-cassar thank you for your patience :) I've been thinking about this a bit and perhaps we could convert to UUID in c++ and avoid awkward Python compat layer? Pandas had a UUID type proposal that seems stalled but could get completed. See my proposal what this could look like. Let me ask for some feedback from others before you continue. |
|
@parker-cassar I think we better implement this in C++ kind of like proposed here. This will likely improve performance and give us a cleaner API/make compat layer with pandas neater. Sorry for not suggesting this sooner. Do you think you can continue in this vein? |
Yes I agree and am happy you suggested this (: the Python compat layer approach doesn't really make sense here, especially since a UUID column in pandas isn't exactly a rare case. Doing it natively is cleaner. I'll give it a go and I'll try to have a PR in a few days or less based on your proposal. |
Rationale for this change
Converting a Table with an
arrow.uuidextension column to pandas currently produces a column ofbytesinstead ofuuid.UUIDobjects. This happens becauseUuidTypedoes not implementto_pandas_dtype(), soTable.to_pandas()falls back to the storage type (fixed_size_binary(16)) and producesbytes. The bug occurs even without a Parquet roundtrip.Note: the original issue suggested this might be specific to Python 3.14 but I tested on Python versions 3.10 - 3.14 and still had the issue since
UuidTypehas never implementedto_pandas_dtype().What changes are included in this PR?
Added
UuidType.to_pandas_dtype(): returns a dtype wrapper implementing__from_arrow__, which delegates toto_pylist()sinceUuidScalar.as_py()already producesuuid.UUIDobjects.Are these changes tested?
Yes. Added
test_uuid_roundtripwhich covers pandas DataFrame with a UUID column -> pyarrow Table -> Parquet on disk -> pyarrow Table -> pandas DataFrame. The final conversion is what this PR fixes.Are there any user-facing changes?
Yes.
Table.to_pandas()now returnsuuid.UUIDforarrow.uuidcolumns instead ofbytes.