refactor: address review feedback on object-dtype handling

Infer object-dtype columns directly with pyarrow instead of sampling and
coercing: strings map to string, other representable scalars to their Arrow
type, an all-null column stays null, and a column mixing incompatible types
raises rather than being silently coerced to string.

Derive the int64 pruning-bound limits from np.iinfo(np.int64) rather than
hardcoded literals, and drop the inaccurate "rare" comments on the cftime
data-variable path.

Author: ghostiee-11

tests/test_df.py

@@ -536,7 +536,7 @@ def test_compute_chunks_tuples_sum_to_dim_size():
         assert sum(tup) == ds.sizes[dim]
 
 
-# -- Object-dtype and out-of-ns-range coordinate support (issue #223) -------
+# -- Object-dtype and out-of-ns-range coordinate support --------------------
 
 
 def _field_type(schema, name):
@@ -584,18 +584,17 @@ def test_partition_metadata_skips_out_of_ns_datetime():
     assert all("time" not in m for m in meta)
 
 
-def test_parse_schema_all_null_object_var_defaults_to_string():
-    # An all-null (or empty) object column has no data to infer from; object
-    # dtype in xarray almost always means strings, so default to pa.string()
-    # rather than a null column.
+def test_parse_schema_all_null_object_var_stays_null():
+    # An all-null object column has no data to infer a type from; let null be
+    # null rather than coercing it to a string column.
     ds = _ensure_default_indexes(
         xr.Dataset(
             {"label": (["x"], np.array([None, None], dtype=object))},
             coords={"x": [1, 2]},
         )
     )
     schema = _parse_schema(ds)
-    assert schema.field("label").type == pa.string()
+    assert pa.types.is_null(schema.field("label").type)
 
 
 def test_partition_metadata_prunes_cftime_coord():

xarray_sql/cftime.py

@@ -170,12 +170,6 @@ def convert_for_field(values, field: pa.Field) -> np.ndarray:
 # ---------------------------------------------------------------------------
 
 
-#: Bounds of a signed 64-bit integer; a pruning bound outside this range
-#: cannot be handed to the Rust ``ScalarBound`` layer.
-_INT64_MIN: int = -(2**63)
-_INT64_MAX: int = 2**63 - 1
-
-
 def partition_bounds(
     values,
 ) -> tuple[int, int, str] | None:
@@ -195,7 +189,8 @@ def partition_bounds(
     if is_gregorian_like(cal):
         us = to_microseconds(values)
         lo, hi = int(us.min()) * 1_000, int(us.max()) * 1_000
-        if lo < _INT64_MIN or hi > _INT64_MAX:
+        int64 = np.iinfo(np.int64)
+        if lo < int64.min or hi > int64.max:
             return None
         return lo, hi, "timestamp_ns"
     offsets = to_offsets(values, DEFAULT_UNITS, cal)

xarray_sql/df.py

@@ -386,29 +386,18 @@ def iter_record_batches(
         yield pa.RecordBatch.from_arrays(arrays, schema=schema)
 
 
-def _arrow_type_for_object(
-    values: np.ndarray, sample_size: int = 100
-) -> pa.DataType:
+def _arrow_type_for_object(values: np.ndarray) -> pa.DataType:
     """Infer an Arrow type for a non-cftime object-dtype array.
 
-    ``pa.from_numpy_dtype`` cannot map numpy object dtype and raises
-    ``ArrowNotImplementedError: Unsupported numpy type 17`` -- which is exactly
-    what a string variable or coordinate produces. Object-dtype arrays are
-    never Dask/Zarr-backed, so letting Arrow infer the type from the data with
-    ``pa.array`` (e.g. ``pa.string()`` for Python strings) is safe and triggers
-    no remote I/O.
-
-    Only the first ``sample_size`` elements are inspected so a huge in-memory
-    object column is not fully copied just to read its type. An empty or
-    all-null sample yields ``pa.null()``, which is meaningless as a column
-    type; object dtype in xarray almost always means strings, so fall back to
-    ``pa.string()`` in that case.
+    ``pa.from_numpy_dtype`` cannot map numpy object dtype, so let pyarrow infer
+    the type from the data instead: strings become ``pa.string()``, bytes
+    ``pa.binary()``, and other representable Python scalars their Arrow
+    equivalent. An all-null array stays ``pa.null()``, and a column mixing
+    incompatible types (e.g. str and int) raises, surfacing a clear error
+    rather than a silent coercion. Object-dtype arrays are never Dask/Zarr
+    backed, so this triggers no remote I/O.
     """
-    sample = np.asarray(values).ravel()[:sample_size]
-    arrow_type = pa.array(sample).type
-    if pa.types.is_null(arrow_type):
-        return pa.string()
-    return arrow_type
+    return pa.array(np.asarray(values).ravel()).type
 
 
 def _parse_schema(ds: xr.Dataset) -> pa.Schema:
@@ -448,19 +437,18 @@ def _parse_schema(ds: xr.Dataset) -> pa.Schema:
                 columns.append(pa.field(coord_name, pa_type))
 
     for var_name, var in ds.data_vars.items():
-        # Data variables are virtually never cftime, but check dtype as a
-        # cheap guard.  Only fall back to _is_cftime (which materializes
-        # element 0) when dtype is object.
+        # An object-dtype data variable may hold cftime objects (encode it like
+        # a cftime coordinate) or strings/other Python scalars (infer the Arrow
+        # type from the data). The dtype check keeps the common numeric path off
+        # the object branch.
         if var.dtype == np.dtype("O"):
             if cft.is_cftime(var.values):
-                # Rare: a data variable holding cftime objects.  Use same
-                # encoding as the first cftime dimension coordinate, or default.
+                # Encode with the same units/calendar as a cftime coordinate.
                 cal = var.values.ravel()[0].calendar
                 columns.append(
                     cft.arrow_field(var_name, cft.DEFAULT_UNITS, cal)
                 )
             else:
-                # String / other object data variable.
                 arrow_type = _arrow_type_for_object(var.values)
                 columns.append(pa.field(var_name, arrow_type))
         else: