jiayuasu commented on code in PR #1195:
URL: https://github.com/apache/sedona-db/pull/1195#discussion_r3911907611
##########
python/sedonadb-geopandas/python/sedonadb_geopandas/_frame.py:
##########
@@ -115,6 +177,262 @@ def __getitem__(self, key):
f"boolean mask, not {type(key).__name__}"
)
+ def __setitem__(self, key, value):
+ """Add or replace a column, as in `gdf["buffered"] =
gdf.geometry.buffer(1)`.
+
+ The underlying frame is immutable, so this rebinds this object to a new
+ frame rather than mutating data in place. A consequence worth knowing:
+ `Series` objects taken from this frame *before* the assignment still
+ refer to the previous frame, so combining one with a column read
+ afterwards raises rather than silently mixing two frames.
+
+ Args:
+ key: Column name to add or replace.
+ value: A `Series`/`GeoSeries` from this same frame, or a scalar
(which
+ may be a geometry) to broadcast to every row.
+
+ A bare SedonaDB expression is deliberately not accepted. An expression
+ carries no record of the frame it was built from, so a column reference
+ taken from another frame would resolve against this one and silently
+ produce this frame's values instead of the intended ones.
+ """
+ if not isinstance(key, str):
+ raise TypeError(f"Column name must be a string, not
{type(key).__name__}")
+
+ from sedonadb.expr import Expr, Literal
+
+ if isinstance(value, Series):
+ if value._df is not self._df:
+ raise ValueError(
+ "Cannot assign a Series that comes from a different "
+ "DataFrame: there is no row alignment, so the result would
"
+ "be silently wrong. Note that assigning to this frame "
+ "rebinds it, so a Series read before an earlier assignment
"
+ "is already stale; re-read it as gdf[...] and try again."
+ )
+ expr = self._series_expr(key, value)
+ elif isinstance(value, Literal):
+ # A literal holds a value rather than a column reference, so there
is
+ # no frame for it to be misattributed to. It still goes through the
+ # scalar path so that a literal geometry gets the same CRS
treatment as
+ # a plain one.
+ expr = self._scalar_expr(key, value)
+ elif isinstance(value, Expr):
+ raise TypeError(
+ "Assigning a bare expression isn't supported: an expression
does "
+ "not record which frame its columns came from, so one built "
+ "against another frame would silently resolve against this
one. "
+ "Assign a Series read from this frame, or a literal."
+ )
+ elif not is_scalar(value):
+ raise TypeError(
+ f"Assigning a {type(value).__name__} isn't supported (there is
no "
+ f"row alignment, so the values could not be matched to rows). "
+ f"Build the column from this frame's own columns, or load the "
+ f"data as a frame and join it."
+ )
+ else:
+ expr = self._scalar_expr(key, value)
+
+ geometry_before = _geometry_column_names(self._df)
+ self._df = self._df.mutate(**{key: expr})
+
+ # Assignment can change whether the active geometry column is still a
+ # geometry: replacing it with a number leaves nothing to be active, and
+ # *creating* a geometry column on a frame without one activates it. The
+ # created-not-preexisting distinction matters: a frame whose geometry
was
+ # explicitly deactivated (geometry=None) must not be reactivated by a
+ # no-op reassignment of a column that was already geometry.
+ geometry_after = _geometry_column_names(self._df)
+ if key == self._geometry_name and key not in geometry_after:
+ self._geometry_name = None
+ elif (
+ self._geometry_name is None
+ and key in geometry_after
+ and key not in geometry_before
+ ):
+ self._geometry_name = key
+
+ def _series_expr(self, key, value):
+ """Adjust a same-frame `Series` expression for assignment to `key`.
+
+ Mirrors the scalar path's CRS rule: a geometry column that carries no
+ CRS of its own inherits the destination column's CRS when it replaces
+ one that has it — GeoPandas keeps the frame CRS in this situation —
+ while a column that carries its own CRS keeps it, since restamping
+ would relabel coordinates without transforming them.
+ """
+ expr = value._expr
+ if key not in _geometry_column_names(self._df):
+ return expr
+ crs = self._df.schema.field(key).type.crs
+ if crs is None or _expr_crs(self._df, expr) is not None:
+ return expr
+ projected = self._df.select(expr.alias("x")).schema
+ if not projected.geometry_column_indices:
+ # A non-geometry value legitimately converts the column.
+ return expr
+ ctx = self._df._ctx
+ return expr.funcs.st_setcrs(ctx.lit(crs.to_json()))
+
+ def _scalar_expr(self, key, value):
+ """Build the expression for broadcasting `value` into column `key`.
+
+ Replacing an existing geometry column keeps that column's type and
CRS, as
+ GeoPandas does. A bare Shapely geometry carries no CRS of its own, and
any
+ missing-value sentinel (`None`, NaN, `pandas.NA`) means "no geometry"
rather
+ than "no longer a geometry column", so neither should silently reset
what
+ the frame already knew.
+
+ A `Literal` is unwrapped and rebuilt on this frame's context: a literal
+ constructed by the bare `lit()` has no context, so functions cannot be
+ applied to it, and passing it straight through would skip the CRS
handling.
+ """
+ from sedonadb.expr import Literal, lit
+
+ from sedonadb_geopandas._series import normalize_scalar
Review Comment:
Done: `sedonadb.expr`, `pyarrow`, and the `_series` helpers are module-level
now (`shapely` too, since it's a declared dependency as of this round), and the
`ImportError` guards that only wrapped pyarrow are gone. numpy and pandas stay
function-local because they're optional.
##########
python/sedonadb-geopandas/python/sedonadb_geopandas/_frame.py:
##########
@@ -92,7 +151,10 @@ def __getitem__(self, key):
# Single column -> (Geo)Series.
if isinstance(key, str):
expr = self._df[key]
- if key == self._geometry_name:
+ # Any geometry-typed column reads back as a GeoSeries — not just
+ # the active one — so a freshly assigned geometry column supports
+ # .area and .buffer() immediately, as it does in GeoPandas.
+ if key == self._geometry_name or key in
_geometry_column_names(self._df):
Review Comment:
The second one is sufficient. The active name is validated to be a geometry
column at construction and cleared whenever the column stops being one, so it's
just the schema check now.
##########
python/sedonadb-geopandas/python/sedonadb_geopandas/_frame.py:
##########
@@ -115,6 +177,262 @@ def __getitem__(self, key):
f"boolean mask, not {type(key).__name__}"
)
+ def __setitem__(self, key, value):
+ """Add or replace a column, as in `gdf["buffered"] =
gdf.geometry.buffer(1)`.
+
+ The underlying frame is immutable, so this rebinds this object to a new
+ frame rather than mutating data in place. A consequence worth knowing:
+ `Series` objects taken from this frame *before* the assignment still
+ refer to the previous frame, so combining one with a column read
+ afterwards raises rather than silently mixing two frames.
+
+ Args:
+ key: Column name to add or replace.
+ value: A `Series`/`GeoSeries` from this same frame, or a scalar
(which
+ may be a geometry) to broadcast to every row.
+
+ A bare SedonaDB expression is deliberately not accepted. An expression
+ carries no record of the frame it was built from, so a column reference
+ taken from another frame would resolve against this one and silently
+ produce this frame's values instead of the intended ones.
+ """
+ if not isinstance(key, str):
+ raise TypeError(f"Column name must be a string, not
{type(key).__name__}")
+
+ from sedonadb.expr import Expr, Literal
+
+ if isinstance(value, Series):
+ if value._df is not self._df:
+ raise ValueError(
+ "Cannot assign a Series that comes from a different "
+ "DataFrame: there is no row alignment, so the result would
"
+ "be silently wrong. Note that assigning to this frame "
+ "rebinds it, so a Series read before an earlier assignment
"
+ "is already stale; re-read it as gdf[...] and try again."
+ )
Review Comment:
Agreed, and it turned out to be tractable without tracking column references
inside expressions. A Series read from a frame now stays valid across
assignments that only *add* columns: the rows and every column it could
reference are unchanged, so its expression resolves by name to the same values.
That covers `g = gdf.geometry; gdf['area'] = g.area; gdf['len'] = g.length;
...`. Replacing a column or filtering still invalidates earlier reads, since a
Series captured before `gdf['v'] = ...` would otherwise resolve to the new `v`.
It's implemented as a list of ancestor frames on the GeoDataFrame rather than a
token on the Series, so `Series` didn't need to change. README updated to say
this.
##########
python/sedonadb-geopandas/python/sedonadb_geopandas/_frame.py:
##########
@@ -115,6 +177,262 @@ def __getitem__(self, key):
f"boolean mask, not {type(key).__name__}"
)
+ def __setitem__(self, key, value):
+ """Add or replace a column, as in `gdf["buffered"] =
gdf.geometry.buffer(1)`.
+
+ The underlying frame is immutable, so this rebinds this object to a new
+ frame rather than mutating data in place. A consequence worth knowing:
+ `Series` objects taken from this frame *before* the assignment still
+ refer to the previous frame, so combining one with a column read
+ afterwards raises rather than silently mixing two frames.
+
+ Args:
+ key: Column name to add or replace.
+ value: A `Series`/`GeoSeries` from this same frame, or a scalar
(which
+ may be a geometry) to broadcast to every row.
+
+ A bare SedonaDB expression is deliberately not accepted. An expression
+ carries no record of the frame it was built from, so a column reference
+ taken from another frame would resolve against this one and silently
+ produce this frame's values instead of the intended ones.
+ """
+ if not isinstance(key, str):
+ raise TypeError(f"Column name must be a string, not
{type(key).__name__}")
+
+ from sedonadb.expr import Expr, Literal
+
+ if isinstance(value, Series):
+ if value._df is not self._df:
+ raise ValueError(
+ "Cannot assign a Series that comes from a different "
+ "DataFrame: there is no row alignment, so the result would
"
+ "be silently wrong. Note that assigning to this frame "
+ "rebinds it, so a Series read before an earlier assignment
"
+ "is already stale; re-read it as gdf[...] and try again."
+ )
+ expr = self._series_expr(key, value)
+ elif isinstance(value, Literal):
+ # A literal holds a value rather than a column reference, so there
is
+ # no frame for it to be misattributed to. It still goes through the
+ # scalar path so that a literal geometry gets the same CRS
treatment as
+ # a plain one.
+ expr = self._scalar_expr(key, value)
+ elif isinstance(value, Expr):
+ raise TypeError(
+ "Assigning a bare expression isn't supported: an expression
does "
+ "not record which frame its columns came from, so one built "
+ "against another frame would silently resolve against this
one. "
+ "Assign a Series read from this frame, or a literal."
+ )
+ elif not is_scalar(value):
+ raise TypeError(
+ f"Assigning a {type(value).__name__} isn't supported (there is
no "
+ f"row alignment, so the values could not be matched to rows). "
+ f"Build the column from this frame's own columns, or load the "
+ f"data as a frame and join it."
+ )
+ else:
+ expr = self._scalar_expr(key, value)
+
+ geometry_before = _geometry_column_names(self._df)
+ self._df = self._df.mutate(**{key: expr})
+
+ # Assignment can change whether the active geometry column is still a
+ # geometry: replacing it with a number leaves nothing to be active, and
+ # *creating* a geometry column on a frame without one activates it. The
+ # created-not-preexisting distinction matters: a frame whose geometry
was
+ # explicitly deactivated (geometry=None) must not be reactivated by a
+ # no-op reassignment of a column that was already geometry.
+ geometry_after = _geometry_column_names(self._df)
+ if key == self._geometry_name and key not in geometry_after:
+ self._geometry_name = None
+ elif (
+ self._geometry_name is None
+ and key in geometry_after
+ and key not in geometry_before
+ ):
+ self._geometry_name = key
+
+ def _series_expr(self, key, value):
+ """Adjust a same-frame `Series` expression for assignment to `key`.
+
+ Mirrors the scalar path's CRS rule: a geometry column that carries no
+ CRS of its own inherits the destination column's CRS when it replaces
+ one that has it — GeoPandas keeps the frame CRS in this situation —
+ while a column that carries its own CRS keeps it, since restamping
+ would relabel coordinates without transforming them.
+ """
+ expr = value._expr
+ if key not in _geometry_column_names(self._df):
+ return expr
+ crs = self._df.schema.field(key).type.crs
+ if crs is None or _expr_crs(self._df, expr) is not None:
+ return expr
+ projected = self._df.select(expr.alias("x")).schema
+ if not projected.geometry_column_indices:
+ # A non-geometry value legitimately converts the column.
+ return expr
+ ctx = self._df._ctx
+ return expr.funcs.st_setcrs(ctx.lit(crs.to_json()))
+
+ def _scalar_expr(self, key, value):
+ """Build the expression for broadcasting `value` into column `key`.
+
+ Replacing an existing geometry column keeps that column's type and
CRS, as
+ GeoPandas does. A bare Shapely geometry carries no CRS of its own, and
any
+ missing-value sentinel (`None`, NaN, `pandas.NA`) means "no geometry"
rather
+ than "no longer a geometry column", so neither should silently reset
what
+ the frame already knew.
+
+ A `Literal` is unwrapped and rebuilt on this frame's context: a literal
+ constructed by the bare `lit()` has no context, so functions cannot be
+ applied to it, and passing it straight through would skip the CRS
handling.
+ """
+ from sedonadb.expr import Literal, lit
+
+ from sedonadb_geopandas._series import normalize_scalar
+
+ raw = value._value if isinstance(value, Literal) else value
+ raw = normalize_scalar(raw)
+
+ # Only geometry values inherit the column's type and CRS. Assigning a
number
+ # over a geometry column is a legitimate way to turn it into an
ordinary
+ # column, and must not be dressed up as geometry. Geometry-ness is
decided
+ # from the resolved literal's schema rather than by duck-typing the
Python
+ # value: a GeoArrow scalar carries no __geo_interface__ yet is
geometry.
+ missing = _is_missing(raw)
+ replacing_geometry = key in _geometry_column_names(self._df)
+
+ # A GeoArrow-typed scalar — valid or null — is recognized from its
+ # extension name, not by resolving it: the scalar resolver drops the
+ # planar/spherical edge type and rejects non-WKB storage outright.
+ # It needs the handling below even for a brand-new or non-geometry
+ # column, so this must come before that early return.
+ geoarrow_typed = False
+ try:
+ import pyarrow as pa
+
+ geoarrow_typed = isinstance(raw, pa.Scalar) and str(
+ getattr(raw.type, "extension_name", "")
+ ).startswith("geoarrow.")
+ except ImportError:
+ pass
+
+ if not replacing_geometry and not geoarrow_typed:
+ return lit(raw)
+
+ if not missing and not geoarrow_typed:
+ candidate = lit(raw)
+ projected = self._df.select(candidate.alias("x")).schema
+ if not projected.geometry_column_indices:
+ return candidate
+
+ if replacing_geometry:
+ dtype = self._df.schema.field(key).type
+ crs = dtype.crs
+ spherical = "SPHERICAL" in str(getattr(dtype, "edge_type",
"")).upper()
+ else:
+ # A new or non-geometry destination has no type or CRS to
+ # inherit; the value's own metadata is all there is.
+ crs = None
+ spherical = False
+ # A context-bound literal is needed to call functions on it.
+ ctx = self._df._ctx
+ inherits_crs = False
+ strips_crs = False
+ expr = None
+ if geoarrow_typed:
+ scalar_spherical = (
+ "SPHERICAL" in str(getattr(raw.type, "edge_type", "")).upper()
+ )
+ scalar_crs = getattr(raw.type, "crs", None)
+ if str(raw.type.extension_name) == "geoarrow.wkb":
+ arr_type = raw.type
+ if pa.types.is_large_binary(arr_type.storage_type):
+ # SedonaDB's WKB importer requires Binary storage; the
+ # type is rebuilt on Binary with the same CRS and edge
Review Comment:
Filed #1215 for the importer. For the broader question, #1214 collects
everything the wrapper currently has to normalize before `lit()` can take it.
The two that matter most here: `_lit_from_geoarrow_scalar` rebuilds the type as
`ga.wkb().with_crs(crs)`, which drops the edge type (spherical scalars come
back planar), and only `WkbScalar` is registered, so WKT and native-point
scalars fail in the `pa.array([obj])` fallback. With those two fixed this whole
GeoArrow branch collapses to `ctx.lit(raw)`. The rebuild stays for now so the
package keeps working on 0.4.1.
##########
python/sedonadb-geopandas/python/sedonadb_geopandas/_frame.py:
##########
@@ -115,6 +177,262 @@ def __getitem__(self, key):
f"boolean mask, not {type(key).__name__}"
)
+ def __setitem__(self, key, value):
+ """Add or replace a column, as in `gdf["buffered"] =
gdf.geometry.buffer(1)`.
+
+ The underlying frame is immutable, so this rebinds this object to a new
+ frame rather than mutating data in place. A consequence worth knowing:
+ `Series` objects taken from this frame *before* the assignment still
+ refer to the previous frame, so combining one with a column read
+ afterwards raises rather than silently mixing two frames.
+
+ Args:
+ key: Column name to add or replace.
+ value: A `Series`/`GeoSeries` from this same frame, or a scalar
(which
+ may be a geometry) to broadcast to every row.
+
+ A bare SedonaDB expression is deliberately not accepted. An expression
+ carries no record of the frame it was built from, so a column reference
+ taken from another frame would resolve against this one and silently
+ produce this frame's values instead of the intended ones.
+ """
+ if not isinstance(key, str):
+ raise TypeError(f"Column name must be a string, not
{type(key).__name__}")
+
+ from sedonadb.expr import Expr, Literal
+
+ if isinstance(value, Series):
+ if value._df is not self._df:
+ raise ValueError(
+ "Cannot assign a Series that comes from a different "
+ "DataFrame: there is no row alignment, so the result would
"
+ "be silently wrong. Note that assigning to this frame "
+ "rebinds it, so a Series read before an earlier assignment
"
+ "is already stale; re-read it as gdf[...] and try again."
+ )
+ expr = self._series_expr(key, value)
+ elif isinstance(value, Literal):
+ # A literal holds a value rather than a column reference, so there
is
+ # no frame for it to be misattributed to. It still goes through the
+ # scalar path so that a literal geometry gets the same CRS
treatment as
+ # a plain one.
+ expr = self._scalar_expr(key, value)
+ elif isinstance(value, Expr):
+ raise TypeError(
+ "Assigning a bare expression isn't supported: an expression
does "
+ "not record which frame its columns came from, so one built "
+ "against another frame would silently resolve against this
one. "
+ "Assign a Series read from this frame, or a literal."
+ )
+ elif not is_scalar(value):
+ raise TypeError(
+ f"Assigning a {type(value).__name__} isn't supported (there is
no "
+ f"row alignment, so the values could not be matched to rows). "
+ f"Build the column from this frame's own columns, or load the "
+ f"data as a frame and join it."
+ )
+ else:
+ expr = self._scalar_expr(key, value)
+
+ geometry_before = _geometry_column_names(self._df)
+ self._df = self._df.mutate(**{key: expr})
+
+ # Assignment can change whether the active geometry column is still a
+ # geometry: replacing it with a number leaves nothing to be active, and
+ # *creating* a geometry column on a frame without one activates it. The
+ # created-not-preexisting distinction matters: a frame whose geometry
was
+ # explicitly deactivated (geometry=None) must not be reactivated by a
+ # no-op reassignment of a column that was already geometry.
+ geometry_after = _geometry_column_names(self._df)
+ if key == self._geometry_name and key not in geometry_after:
+ self._geometry_name = None
+ elif (
+ self._geometry_name is None
+ and key in geometry_after
+ and key not in geometry_before
+ ):
+ self._geometry_name = key
+
+ def _series_expr(self, key, value):
+ """Adjust a same-frame `Series` expression for assignment to `key`.
+
+ Mirrors the scalar path's CRS rule: a geometry column that carries no
+ CRS of its own inherits the destination column's CRS when it replaces
+ one that has it — GeoPandas keeps the frame CRS in this situation —
+ while a column that carries its own CRS keeps it, since restamping
+ would relabel coordinates without transforming them.
+ """
+ expr = value._expr
+ if key not in _geometry_column_names(self._df):
+ return expr
+ crs = self._df.schema.field(key).type.crs
+ if crs is None or _expr_crs(self._df, expr) is not None:
+ return expr
+ projected = self._df.select(expr.alias("x")).schema
+ if not projected.geometry_column_indices:
+ # A non-geometry value legitimately converts the column.
+ return expr
+ ctx = self._df._ctx
+ return expr.funcs.st_setcrs(ctx.lit(crs.to_json()))
+
+ def _scalar_expr(self, key, value):
+ """Build the expression for broadcasting `value` into column `key`.
+
+ Replacing an existing geometry column keeps that column's type and
CRS, as
+ GeoPandas does. A bare Shapely geometry carries no CRS of its own, and
any
+ missing-value sentinel (`None`, NaN, `pandas.NA`) means "no geometry"
rather
+ than "no longer a geometry column", so neither should silently reset
what
+ the frame already knew.
+
+ A `Literal` is unwrapped and rebuilt on this frame's context: a literal
+ constructed by the bare `lit()` has no context, so functions cannot be
+ applied to it, and passing it straight through would skip the CRS
handling.
+ """
+ from sedonadb.expr import Literal, lit
+
+ from sedonadb_geopandas._series import normalize_scalar
+
+ raw = value._value if isinstance(value, Literal) else value
+ raw = normalize_scalar(raw)
+
+ # Only geometry values inherit the column's type and CRS. Assigning a
number
+ # over a geometry column is a legitimate way to turn it into an
ordinary
+ # column, and must not be dressed up as geometry. Geometry-ness is
decided
+ # from the resolved literal's schema rather than by duck-typing the
Python
+ # value: a GeoArrow scalar carries no __geo_interface__ yet is
geometry.
+ missing = _is_missing(raw)
+ replacing_geometry = key in _geometry_column_names(self._df)
+
+ # A GeoArrow-typed scalar — valid or null — is recognized from its
+ # extension name, not by resolving it: the scalar resolver drops the
+ # planar/spherical edge type and rejects non-WKB storage outright.
+ # It needs the handling below even for a brand-new or non-geometry
+ # column, so this must come before that early return.
+ geoarrow_typed = False
+ try:
+ import pyarrow as pa
+
+ geoarrow_typed = isinstance(raw, pa.Scalar) and str(
+ getattr(raw.type, "extension_name", "")
+ ).startswith("geoarrow.")
+ except ImportError:
+ pass
+
+ if not replacing_geometry and not geoarrow_typed:
+ return lit(raw)
+
+ if not missing and not geoarrow_typed:
+ candidate = lit(raw)
+ projected = self._df.select(candidate.alias("x")).schema
+ if not projected.geometry_column_indices:
+ return candidate
+
+ if replacing_geometry:
+ dtype = self._df.schema.field(key).type
+ crs = dtype.crs
+ spherical = "SPHERICAL" in str(getattr(dtype, "edge_type",
"")).upper()
+ else:
+ # A new or non-geometry destination has no type or CRS to
+ # inherit; the value's own metadata is all there is.
+ crs = None
+ spherical = False
+ # A context-bound literal is needed to call functions on it.
+ ctx = self._df._ctx
+ inherits_crs = False
+ strips_crs = False
+ expr = None
+ if geoarrow_typed:
+ scalar_spherical = (
+ "SPHERICAL" in str(getattr(raw.type, "edge_type", "")).upper()
+ )
+ scalar_crs = getattr(raw.type, "crs", None)
+ if str(raw.type.extension_name) == "geoarrow.wkb":
+ arr_type = raw.type
+ if pa.types.is_large_binary(arr_type.storage_type):
+ # SedonaDB's WKB importer requires Binary storage; the
+ # type is rebuilt on Binary with the same CRS and edge
+ # metadata rather than passed through and rejected.
+ import geoarrow.pyarrow as ga
+
+ rebuilt = ga.wkb().with_edge_type(arr_type.edge_type)
+ if arr_type.crs is not None:
+ rebuilt = rebuilt.with_crs(arr_type.crs)
+ arr_type = rebuilt
+ expr = ctx.lit(pa.array([raw.as_py()], type=arr_type))
+ elif missing:
+ # Non-WKB storage cannot become a literal; a null of it is
+ # rebuilt from its own metadata. Kind and CRS survive; the
+ # storage kind, which holds nothing for a null, does not.
+ if scalar_spherical:
+ expr = ctx.lit(None).funcs.st_geogfromwkt()
+ else:
+ expr = ctx.lit(None).funcs.st_geomfromwkt()
+ if scalar_crs:
+ # GeoArrow CRS wrappers stringify as StringCrs(...);
+ # to_json() is the canonical PROJJSON form ST_SetCRS
+ # accepts.
+ crs_text = (
+ scalar_crs.to_json()
+ if hasattr(scalar_crs, "to_json")
+ else str(scalar_crs)
+ )
+ expr = expr.funcs.st_setcrs(ctx.lit(crs_text))
+ else:
+ # A CRS-less carrier inherits the destination CRS like
+ # any other, shedding any constructor-synthesized one.
+ inherits_crs = True
+ strips_crs = crs is None
+ else:
+ # A valid non-WKB GeoArrow scalar keeps the literal
+ # resolver's own error, which names the unsupported storage.
+ expr = ctx.lit(raw)
+ if expr is not None:
+ pass
+ elif missing:
+ # The typed null is built with the destination's own spatial kind:
+ # a geography column stays geography rather than degrading to
+ # planar geometry. A missing value has no CRS of its own, whatever
+ # CRS the constructor synthesizes — so a CRS-less destination
+ # strips the synthesized one back off.
+ inherits_crs = True
+ if spherical:
+ expr = ctx.lit(None).funcs.st_geogfromwkt()
+ strips_crs = crs is None
+ else:
+ expr = ctx.lit(None).funcs.st_geomfromwkt()
+ elif spherical:
+ try:
+ from shapely.geometry.base import BaseGeometry
+ except ImportError:
+ BaseGeometry = ()
+ if isinstance(raw, BaseGeometry):
+ # A bare Shapely value re-enters through WKB as geography.
+ # It carries no CRS of its own — the constructor synthesizes
+ # CRS84 — so the destination CRS applies. (A value that
+ # already carries a spatial type — a GeoArrow scalar, say —
+ # keeps it; converting between planar and spherical semantics
+ # is not something an assignment should do silently.)
+ inherits_crs = True
+ strips_crs = crs is None
+ expr = ctx.lit(raw.wkb).funcs.st_geogfromwkb()
+ else:
+ expr = ctx.lit(raw)
+ else:
+ expr = ctx.lit(raw)
+ # The destination CRS is inherited by values that have none of their
+ # own: missing values, bare Shapely geometry, and anything whose
+ # projection shows no CRS. A value that carries its own CRS (a
+ # GeoSeries literal, say) keeps it: stamping the destination CRS over
+ # it would relabel the coordinates without transforming them, which
+ # is silently wrong data.
+ if crs is not None and (inherits_crs or _expr_crs(self._df, expr) is
None):
+ expr = expr.funcs.st_setcrs(ctx.lit(crs.to_json()))
+ elif strips_crs and _expr_crs(self._df, expr) is not None:
+ # SRID 0 means "no CRS" and keeps the value; st_setcrs(NULL)
+ # would null-propagate and erase every row.
+ expr = expr.funcs.st_setsrid(ctx.lit(0))
Review Comment:
Yes, SetSRID(0) is what's used here. It's the value-preserving spelling;
`ST_SetCRS(NULL)` null-propagated and erased the rows, which is how this branch
came to exist.
##########
python/sedonadb-geopandas/python/sedonadb_geopandas/_series.py:
##########
@@ -17,6 +17,191 @@
"""pandas/GeoPandas-style Series backed by a SedonaDB expression."""
+def is_scalar(value):
+ """Whether `value` is a single value that can be broadcast to every row.
+
+ Checking for `__array__` alone is not enough in either direction: a list or
+ tuple has no `__array__` yet is a sequence, while a NumPy scalar has one
and
+ *is* a single value. So sequences are rejected explicitly, and anything
+ array-like is judged by its dimensionality — 0-d is a scalar, anything else
+ holds multiple values and has no defined row alignment here.
+
+ Shared by operators and assignment so the two cannot disagree about what
+ counts as a scalar.
+ """
+ if isinstance(value, (str, bytes, bytearray)):
+ return True
+ try:
+ import pyarrow as pa
+
+ # An Arrow scalar is one value even when it implements __len__ (a
+ # ListScalar's length is its element count, not a row count).
+ if isinstance(value, pa.Scalar):
+ return True
+ except ImportError:
+ pass
+ try:
+ from shapely.geometry.base import BaseGeometry
+
+ # Shapely 1.x multipart geometries implement __len__ and __iter__;
+ # they are still single values. (Shapely 2 removed the sequence
+ # protocol, but the package floor does not require Shapely 2.)
Review Comment:
Done: `shapely>=2` is now a declared dependency. It wasn't declared at all
before and only came in through the geopandas extra. The 1.x multipart guard
and its test are gone.
##########
python/sedonadb-geopandas/python/sedonadb_geopandas/_series.py:
##########
@@ -17,6 +17,191 @@
"""pandas/GeoPandas-style Series backed by a SedonaDB expression."""
+def is_scalar(value):
+ """Whether `value` is a single value that can be broadcast to every row.
+
+ Checking for `__array__` alone is not enough in either direction: a list or
+ tuple has no `__array__` yet is a sequence, while a NumPy scalar has one
and
+ *is* a single value. So sequences are rejected explicitly, and anything
+ array-like is judged by its dimensionality — 0-d is a scalar, anything else
+ holds multiple values and has no defined row alignment here.
+
+ Shared by operators and assignment so the two cannot disagree about what
+ counts as a scalar.
+ """
+ if isinstance(value, (str, bytes, bytearray)):
+ return True
+ try:
+ import pyarrow as pa
+
+ # An Arrow scalar is one value even when it implements __len__ (a
+ # ListScalar's length is its element count, not a row count).
+ if isinstance(value, pa.Scalar):
+ return True
+ except ImportError:
+ pass
+ try:
+ from shapely.geometry.base import BaseGeometry
+
+ # Shapely 1.x multipart geometries implement __len__ and __iter__;
+ # they are still single values. (Shapely 2 removed the sequence
+ # protocol, but the package floor does not require Shapely 2.)
+ if isinstance(value, BaseGeometry):
+ return True
+ except ImportError:
+ pass
+ if isinstance(value, (list, tuple, set, frozenset, dict, range)):
+ return False
+ if hasattr(value, "__array__"):
+ return getattr(value, "ndim", None) == 0
+ # Non-sequence objects (numbers, shapely geometries, None, ...) broadcast.
+ return not hasattr(value, "__len__")
+
+
+def normalize_scalar(value):
+ """Normalize an accepted scalar into something a literal can hold.
+
+ Passing the classifier is not the same as being constructible: a 0-d NumPy
+ array is a scalar but `lit()` cannot take it, and `pandas.NA` is a missing
+ sentinel `lit()` does not recognize. Unwrap the former to its Python value
+ and convert missing sentinels to `None` (SQL null). Callers apply this only
+ after `is_scalar` has accepted the value.
Review Comment:
Agreed, that's #1214. It lists each case with today's behavior (`pd.NA` and
`NaT` unrecognized, `Timestamp`/`Timedelta` silently truncated to microseconds,
numpy day/week/year units rejected, `np.ma.masked`, 0-d arrays, structured
voids, typed-null nested scalars) and notes that the class-name table already
handles the pandas cases without importing pandas. Each one that lands upstream
lets `normalize_scalar` shrink to a pass-through.
##########
python/sedonadb-geopandas/tests/test_geopandas_compat.py:
##########
@@ -242,3 +252,777 @@ def test_head(cities):
assert len(gdf.head(2)) == 2
# head() keeps the active geometry column.
assert isinstance(gdf.head(2).geometry, GeoSeries)
+
+
+# -- column assignment -------------------------------------------------------
+
+
+def test_setitem_from_series(points):
+ gdf = sgpd.from_geopandas(points)
+ gdf["v2"] = gdf["v"]
+ got = gdf.to_geopandas().sort_values("name")
+ assert got["v2"].tolist() == points["v"].tolist()
+
+
+def test_setitem_replaces_existing_column(points):
+ gdf = sgpd.from_geopandas(points)
+ gdf["v"] = gdf["name"]
+ assert sorted(gdf.to_geopandas()["v"]) == ["A", "B", "C"]
+ assert gdf.columns.count("v") == 1
+
+
+def test_setitem_scalar_broadcasts(points):
+ gdf = sgpd.from_geopandas(points)
+ gdf["k"] = 7
+ assert gdf.to_geopandas()["k"].tolist() == [7, 7, 7]
+
+
+def test_setitem_geometry_column(points):
+ gdf = sgpd.from_geopandas(points)
+ gdf["buffered"] = gdf.geometry.buffer(0.5)
+ assert "buffered" in gdf.columns
+ # The active geometry column is unchanged by adding another one.
+ assert gdf.geometry._name == "geometry"
+
+
+def test_setitem_makes_geometry_active_when_frame_had_none():
+ # A frame that starts without geometry gains an active geometry column when
+ # one is assigned.
+ from shapely.geometry import Point
+
+ plain = GeoDataFrame(sgpd.default_context().sql("SELECT 1 AS a"))
+ assert plain._geometry_name is None
+
+ plain["geometry"] = Point(1, 2)
+ assert plain._geometry_name == "geometry"
+ assert plain.to_geopandas().geometry.to_wkt().tolist() == ["POINT (1 2)"]
+
+
+def test_setitem_non_geometry_leaves_frame_without_geometry():
+ plain = GeoDataFrame(sgpd.default_context().sql("SELECT 1.0 AS x"))
+ plain["doubled"] = plain["x"]
+ assert plain._geometry_name is None
+
+
+def test_setitem_rejects_bad_inputs(points):
+ gdf = sgpd.from_geopandas(points)
+ with pytest.raises(TypeError, match="must be a string"):
+ gdf[0] = 1
+ with pytest.raises(TypeError, match="isn't supported"):
+ gdf["x"] = points["v"]
+ # A Series from a different frame has no row alignment.
+ other = sgpd.from_geopandas(points)
+ with pytest.raises(ValueError, match="different"):
+ gdf["y"] = other["v"]
+
+
+def test_setitem_stale_series_raises(points):
+ # Assignment rebinds the frame, so a Series read beforehand is stale.
+ gdf = sgpd.from_geopandas(points)
+ before = gdf["v"]
+ gdf["k"] = 1
+ with pytest.raises(ValueError, match="different"):
+ gdf["z"] = before
+
+
+def test_setitem_rejects_bare_expression(points):
+ # A bare expression carries no origin, so one built from another frame
would
+ # resolve against this frame and silently write this frame's values.
+ left = sgpd.from_geopandas(points)
+ right = sgpd.from_geopandas(points)
+ with pytest.raises(TypeError, match="bare expression"):
+ left["copied"] = right["v"]._expr
+
+
+def test_series_has_no_unguarded_expr_escape_hatch(points):
+ assert not hasattr(sgpd.from_geopandas(points)["v"], "expr")
+
+
[email protected](
+ "value",
+ [[10, 20, 30], (10, 20, 30), {10, 20}],
+ ids=["list", "tuple", "set"],
+)
+def test_setitem_rejects_sequences(points, value):
+ # Sequences have no __array__, so they used to be broadcast whole into
every
+ # row rather than rejected.
+ gdf = sgpd.from_geopandas(points)
+ with pytest.raises(TypeError, match="isn't supported"):
+ gdf["x"] = value
+
+
+def test_setitem_accepts_numpy_scalar(points):
+ # NumPy scalars do have __array__ but are single values, so they used to be
+ # rejected as array-likes.
+ import numpy as np
+
+ gdf = sgpd.from_geopandas(points)
+ gdf["x"] = np.int64(5)
+ assert gdf.to_geopandas()["x"].tolist() == [5, 5, 5]
+
+
+def test_setitem_rejects_numpy_array(points):
+ import numpy as np
+
+ gdf = sgpd.from_geopandas(points)
+ with pytest.raises(TypeError, match="isn't supported"):
+ gdf["x"] = np.array([1, 2, 3])
+
+
+def test_getitem_rejects_stale_mask(points):
+ # Assignment rebinds the frame, so a mask captured beforehand belongs to
the
+ # previous one. It used to be accepted and quietly resolve against the new
+ # frame while the referenced column happened to still exist.
+ gdf = sgpd.from_geopandas(points)
+ mask = gdf["v"] > 1
+ gdf["k"] = 1
+ with pytest.raises(ValueError, match="different DataFrame"):
+ gdf[mask]
+
+
+def test_setitem_none_keeps_geometry_column(points):
+ # Assigning None used to turn the column untyped and clear the active
geometry;
+ # GeoPandas keeps a geometry column with its CRS.
+ gdf = sgpd.from_geopandas(points)
+ gdf["geometry"] = None
+ assert gdf._geometry_name == "geometry"
+ assert "3857" in str(gdf.crs)
+ assert gdf.to_geopandas().geometry.isna().all()
+
+
+def test_setitem_non_geometry_over_geometry_still_clears(points):
+ # The CRS-preserving path must not dress a number up as geometry.
+ gdf = sgpd.from_geopandas(points)
+ gdf["geometry"] = 7
+ assert gdf._geometry_name is None
+ assert gdf.to_geopandas()["geometry"].tolist() == [7, 7, 7]
+
+
[email protected](
+ "value_name",
+ ["none", "nan", "pandas_na", "geometry", "literal_geometry",
"literal_none"],
+)
+def test_setitem_geometry_scalars_keep_type_and_crs(points, value_name):
+ # Every supported scalar path has to go through the CRS-preserving branch:
+ # GeoPandas treats None, NaN and pd.NA as missing geometry and keeps the
typed
+ # column and its CRS.
+ import numpy as np
+ import pandas as pd
+ from shapely.geometry import Point
+
+ from sedonadb.expr import lit
+
+ values = {
+ "none": None,
+ "nan": np.nan,
+ "pandas_na": pd.NA,
+ "geometry": Point(5, 5),
+ "literal_geometry": lit(Point(5, 5)),
+ "literal_none": lit(None),
+ }
+ gdf = sgpd.from_geopandas(points)
+ gdf["geometry"] = values[value_name]
+ assert gdf._geometry_name == "geometry"
+ assert "3857" in str(gdf.crs)
+
+
+def test_setitem_crs_carrying_literal_keeps_its_crs(points):
+ # A literal that carries its own CRS must not be relabeled with the
+ # destination column's CRS — that changes what the coordinates mean without
+ # transforming them.
+ from sedonadb.expr import lit
+
+ src = gpd.GeoSeries.from_wkt(["POINT (10 10)"], crs="EPSG:4326")
+ gdf = sgpd.from_geopandas(points) # column is EPSG:3857
+ gdf["geometry"] = lit(src)
+ assert "4326" in str(gdf.crs)
+
+
+def test_pandas_na_assigns_as_null_to_ordinary_column(points):
+ import pandas as pd
+
+ gdf = sgpd.from_geopandas(points)
+ gdf["z"] = pd.NA
+ assert gdf.to_geopandas()["z"].isna().all()
+
+
+def test_zero_dimensional_array_normalizes(points):
+ import numpy as np
+
+ gdf = sgpd.from_geopandas(points)
+ gdf["z"] = np.array(5) # 0-d: scalar by classification, unwrapped on use
+ assert gdf.to_geopandas()["z"].tolist() == [5, 5, 5]
+
+
+def test_masked_scalar_assigns_as_missing(points):
+ import numpy as np
+
+ gdf = sgpd.from_geopandas(points)
+ gdf["m"] = np.ma.masked
+ assert gdf.to_geopandas()["m"].isna().all()
+
+
+def test_pyarrow_scalars_broadcast(points):
+ # Arrow scalars implement __len__ but are single values.
+ import pyarrow as pa
+
+ from sedonadb_geopandas._series import is_scalar
+
+ assert is_scalar(pa.scalar([1, 2]))
+ assert is_scalar(pa.scalar({"a": 1}))
+ gdf = sgpd.from_geopandas(points)
+ gdf["tags"] = pa.scalar([1, 2])
+ assert len(gdf.to_geopandas()["tags"]) == 3
+
+
+def test_geoarrow_scalar_inherits_crs(points):
+ # A GeoArrow WKB scalar has no __geo_interface__, so geometry-ness must
come
+ # from the resolved schema; it is CRS-less and inherits the column's CRS.
+ import geoarrow.pyarrow as ga
+
+ w = ga.as_wkb(ga.array(["POINT (5 5)"]))[0]
+ gdf = sgpd.from_geopandas(points)
+ gdf["geometry"] = w
+ assert gdf._geometry_name == "geometry"
+ assert "3857" in str(gdf.crs)
+
+
+def test_explicitly_inactive_geometry_stays_inactive(points):
+ # geometry=None is a choice; a no-op reassignment of an existing geometry
+ # column must not silently reactivate it.
+ df = sgpd.default_context().create_data_frame(points)
+ gdf = GeoDataFrame(df, geometry=None)
+ gdf["geometry"] = gdf["geometry"]
+ assert gdf._geometry_name is None
+
+
+def test_temporal_scalars_are_deferred(points):
+ # Representing temporal scalars faithfully needs dedicated unit and
+ # timezone handling — a naive literal would silently truncate nanoseconds
+ # or reject most NumPy units — which arrives as its own change; until
+ # then they are rejected outright rather than stored subtly wrong.
+ import numpy as np
+ import pandas as pd
+
+ gdf = sgpd.from_geopandas(points)
+ for value in (
+ np.datetime64("2026-01-01", "ns"),
+ np.timedelta64(1, "ns"),
+ pd.Timestamp("2026-01-01"),
+ pd.Timedelta(1),
+ ):
+ with pytest.raises(TypeError, match="not supported yet"):
+ gdf["t"] = value
+
+
+# -- regressions from review ------------------------------------------------
+
+
+def test_assigned_geometry_column_reads_back_as_geoseries(points):
+ # Only the active geometry name produced a GeoSeries, so the advertised
+ # gdf["buffered"] = gdf.geometry.buffer(...) gave back a plain Series
+ # with no .area or .buffer(). Geometry-ness comes from the schema.
+ gdf = sgpd.from_geopandas(points)
+ gdf["buffered"] = gdf.geometry.buffer(0.5)
+ col = gdf["buffered"]
+ assert isinstance(col, type(gdf.geometry))
+ assert (col.area.to_pandas() > 0).all()
+
+
+def test_cleared_geometry_is_not_resurrected_by_materialization(points):
+ # With the active geometry replaced by a number, the wrapper records no
+ # active geometry — but the materializer heuristically activated any
+ # remaining geometry column, so a later to_crs() on the result silently
+ # targeted a column this frame never had active.
+ gdf = sgpd.from_geopandas(points)
+ gdf["copy"] = gdf.geometry
+ gdf["geometry"] = 7
+ assert gdf._geometry_name is None
+ out = gdf.to_geopandas()
+ with pytest.raises(AttributeError):
+ out.geometry
+
+
+def test_series_assignment_inherits_destination_crs():
+ # Assigning a CRS-less same-frame geometry column over a column that has
+ # a CRS dropped it; GeoPandas keeps the frame CRS. A column carrying its
+ # own CRS keeps it instead.
+ gdf = GeoDataFrame(
+ sgpd.default_context().sql(
+ "SELECT ST_SetSRID(ST_Point(0.0, 0.0), 3857) AS geometry, "
+ "ST_Point(5.0, 5.0) AS bare, "
+ "ST_SetSRID(ST_Point(7.0, 7.0), 4326) AS own, 1 AS v"
+ )
+ )
+ gdf["geometry"] = gdf["bare"]
+ assert "3857" in str(gdf.crs)
+ gdf["geometry"] = gdf["own"]
+ # The engine reports SRID 4326 as OGC:CRS84; the point is that the
+ # source's own CRS survives instead of being restamped with 3857.
+ assert gdf.crs is not None
+ assert "3857" not in str(gdf.crs)
+
+
+def test_geography_column_replacement_preserves_geography():
+ # Replacing a geography column with None or a Shapely scalar rebuilt it
+ # as planar geometry; replacements are constructed with the destination's
+ # own spatial kind.
+ from shapely.geometry import Point
+
+ def geog_frame():
+ return GeoDataFrame(
+ sgpd.default_context().sql(
+ "SELECT ST_GeogFromWKT('POINT (0 0)') AS g, 1 AS v"
+ ),
+ geometry="g",
+ )
+
+ gdf = geog_frame()
+ gdf["g"] = None
+ assert "geography" in str(gdf._df.schema.field("g").type)
+ assert gdf._geometry_name == "g"
+ gdf = geog_frame()
+ gdf["g"] = Point(1, 1)
+ assert "geography" in str(gdf._df.schema.field("g").type)
+ got = gdf.to_geopandas()["g"]
+ assert got.tolist()[0] == Point(1, 1)
+
+ # A non-default CRS survives too: the geography constructors synthesize
+ # CRS84, which must not be mistaken for a CRS the value carried itself.
+ def geog_4267():
+ return GeoDataFrame(
+ sgpd.default_context().sql(
+ "SELECT ST_SetSRID(ST_GeogFromWKT('POINT (0 0)'), 4267) AS g,
1 AS v"
+ ),
+ geometry="g",
+ )
+
+ for value in (None, Point(1, 1)):
+ gdf = geog_4267()
+ gdf["g"] = value
+ dtype = str(gdf._df.schema.field("g").type)
+ assert "geography" in dtype
+ assert "4267" in dtype
+
+
+def test_numpy_scalar_dtypes_are_preserved(points):
+ # .item() promoted np.int8/np.float32 to int64/float64 columns and
+ # overflowed np.uint64 past int64, which the engine supports natively.
+ import numpy as np
Review Comment:
Done: numpy, pyarrow, pandas, shapely, geoarrow and `lit` are module-level
in the test file now.
--
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]
