tritsystem commented on PR #51167: URL: https://github.com/apache/arrow/pull/51167#issuecomment-6307577586
Thanks for the review — done, and it turned up something worth flagging. Switched to `continue` (dropped the schema-field fallback entirely), so an unparseable `numpy_type` string no longer gets treated as "must be an ArrowDtype" — the column just falls back to its normal, non-metadata conversion. While doing that I also broadened `except TypeError` to `except (TypeError, ValueError)`. Traced it down: `pandas_dtype()` tries `np.dtype()` before the ArrowDtype parser, and for any metadata string with a comma in it — struct, dictionary, map, anything with more than one field — `np.dtype()` misreads the comma as its own legacy multi-field format syntax and raises `ValueError`, never reaching the code path that raises `TypeError` for a comma-free string like the list case. So the original `except TypeError` only ever caught the list case; struct/dictionary/map were still crashing. Confirmed directly against `pandas_dtype()`/`np.dtype()` on pandas 2.3.3 (both before and after this change), not just inferred. Updated the test to match the new behavior: it now checks that the column lands as `object` (list/struct) or `category` (dictionary) with no crash, rather than asserting an exact ArrowDtype round-trip, since that's no longer what this fix does. Added a dictionary case too, since it's the same ValueError path. (Disclosure: I used Claude Code to trace the TypeError/ValueError split, rewrite the fix and test, and draft this comment; I reviewed the change and ran it myself before pushing.) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
