Copilot commented on code in PR #51163:
URL: https://github.com/apache/arrow/pull/51163#discussion_r4528741315
##########
python/pyarrow/_parquet.pyx:
##########
@@ -1663,6 +1663,8 @@ cdef class ParquetReader(_Weakrefable):
properties.set_page_checksum_verification(page_checksum_verification)
if binary_type is not None:
+ if not isinstance(binary_type, DataType):
+ raise TypeError(f"DataType expected, got
{type(binary_type)!r}")
c_binary_type = pyarrow_unwrap_data_type(binary_type)
arrow_props.set_binary_type(c_binary_type.get().id())
Review Comment:
The `DataType` validation logic and error message added here is duplicated
with the similar check in `python/pyarrow/_dataset_parquet.pyx`
(`ParquetReadOptions.binary_type`). Consider factoring this into a small shared
helper (or reusing an existing internal type-checking utility) so future
message/behavior changes stay consistent across both code paths.
##########
python/pyarrow/_compute.pyx:
##########
@@ -1681,7 +1681,7 @@ class IndexOptions(_IndexOptions):
The value to search for.
"""
- def __init__(self, value):
+ def __init__(self, Scalar value not None):
self._set_options(value)
Review Comment:
Tightening `IndexOptions` to require a `pyarrow.Scalar` will now reject
common Python scalar inputs (e.g., `0`, `"x"`) rather than converting them. If
this API previously accepted scalar-like Python values (or is expected to for
usability/compatibility), consider accepting `object value` and explicitly
converting via the existing scalar conversion utilities, while still raising
`TypeError` for unsupported values to avoid native crashes.
##########
python/pyarrow/_acero.pyx:
##########
@@ -565,6 +565,8 @@ cdef class Declaration(_Weakrefable):
if inputs is not None:
for ipt in inputs:
+ if not isinstance(ipt, Declaration):
+ raise TypeError("Expected a Declaration")
c_inputs.push_back(
CDeclaration.Input((<Declaration>ipt).unwrap())
Review Comment:
The new `TypeError("Expected a Declaration")` is quite generic and doesn’t
identify which argument failed (e.g., `inputs`) or what type was received.
Consider including the parameter name and the offending type (and possibly
index) to make debugging easier, e.g., `"inputs must contain Declaration
objects, got <class ...>"` (and keep tests aligned to the improved message).
--
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]
