jiayuasu commented on code in PR #1052: URL: https://github.com/apache/sedona-db/pull/1052#discussion_r3654771509
########## python/sedonadb-geopandas/python/sedonadb_geopandas/_frame.py: ########## @@ -0,0 +1,90 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""GeoPandas-style GeoDataFrame backed by a lazy SedonaDB frame.""" + +from sedonadb_geopandas._series import GeoSeries, Series + + +class GeoDataFrame: + """A lazy SedonaDB frame in the shape of a ``geopandas.GeoDataFrame``. + + Wraps a SedonaDB ``DataFrame`` and tracks the active geometry column. + Row selection, column access, and geometry operations mirror GeoPandas but + build a query rather than computing eagerly; call ``to_geopandas()`` to + materialize. + """ + + def __init__(self, df, geometry="geometry"): + self._df = df + self._geometry_name = geometry + + @property + def geometry(self): + """The active geometry column as a :class:`GeoSeries`.""" + return GeoSeries(self._df, self._df[self._geometry_name], self._geometry_name) + + @property + def crs(self): + """The CRS of the active geometry column (via a zero-row materialization).""" + return self._df.limit(0).to_pandas().crs + + @property + def columns(self): + """Column names, mirroring ``GeoDataFrame.columns``.""" + return list(self._df.schema.names) + + def __getitem__(self, key): + # Boolean mask -> row filter (gdf[gdf["pop"] > 1000]). + if isinstance(key, Series): + return GeoDataFrame(self._df.filter(key._expr), self._geometry_name) + + # Column subset -> GeoDataFrame (keeps the geometry column if selected). + if isinstance(key, list): + return GeoDataFrame(self._df.select(*key), self._geometry_name) + + # Single column -> (Geo)Series. + if isinstance(key, str): + expr = self._df[key] + if key == self._geometry_name: + return GeoSeries(self._df, expr, key) + return Series(self._df, expr, key) + + raise TypeError( + f"GeoDataFrame indices must be a column name, list of names, or " + f"boolean mask, not {type(key).__name__}" + ) + + def to_crs(self, crs): + """Reproject the geometry column to ``crs`` (``ST_Transform``).""" + from sedonadb.expr import lit + + transformed = self._df[self._geometry_name].geo.transform(lit(str(crs))) + new_df = self._df.mutate(**{self._geometry_name: transformed}) + return GeoDataFrame(new_df, self._geometry_name) + + def to_geopandas(self): + """Execute and return a ``geopandas.GeoDataFrame``.""" + return self._df.to_pandas() + + # Alias: results carry geometry, so this returns a GeoDataFrame too. + to_pandas = to_geopandas + + def __len__(self): + return self._df.count() + + def __repr__(self): + return repr(self.to_geopandas()) Review Comment: Fixed — `__repr__` no longer collects at all; it reports the columns and the active geometry column. Added a `_repr_html_` for Jupyter that collects a bounded preview (up to 10 rows) via `limit()`, falling back to `__repr__` if that fails. A test asserts the repr does not execute. ########## python/sedonadb-geopandas/python/sedonadb_geopandas/_series.py: ########## @@ -0,0 +1,109 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""pandas/GeoPandas-style Series backed by a SedonaDB expression.""" + + +def _operand(other): + """Unwrap a Series to its expression; pass scalars through unchanged.""" + return other._expr if isinstance(other, Series) else other + + +class Series: + """A single column of a lazy SedonaDB frame, in the shape of a pandas Series. + + A ``Series`` pairs a source SedonaDB ``DataFrame`` with an expression over + its columns. Comparisons produce boolean ``Series`` usable as a filter mask + (``gdf[gdf["pop"] > 1000]``). Nothing is computed until ``to_pandas()`` / + display. + """ + + def __init__(self, df, expr, name): + self._df = df + self._expr = expr + self._name = name + + # -- element-wise comparisons -> boolean mask -------------------------- + def __gt__(self, other): + return Series(self._df, self._expr > _operand(other), self._name) + + def __ge__(self, other): + return Series(self._df, self._expr >= _operand(other), self._name) + + def __lt__(self, other): + return Series(self._df, self._expr < _operand(other), self._name) + + def __le__(self, other): + return Series(self._df, self._expr <= _operand(other), self._name) + + def __eq__(self, other): + return Series(self._df, self._expr == _operand(other), self._name) + + def __ne__(self, other): + return Series(self._df, self._expr != _operand(other), self._name) + + # -- boolean composition of masks -------------------------------------- + def __and__(self, other): + return Series(self._df, self._expr & _operand(other), self._name) + + def __or__(self, other): + return Series(self._df, self._expr | _operand(other), self._name) + + def __invert__(self): + return Series(self._df, ~self._expr, self._name) + + __hash__ = None + + # -- materialization --------------------------------------------------- + def to_pandas(self): + """Execute and return this column as a pandas (or GeoPandas) Series.""" + return self._df.select(self._expr.alias(self._name)).to_pandas()[self._name] + + def __repr__(self): + return repr(self.to_pandas()) Review Comment: Fixed — `Series.__repr__` now shows the underlying expression rather than executing, with a hint to collect: `<GeoSeries Expr(st_centroid(geometry)) (lazy; call .to_pandas())>`. ########## python/sedonadb-geopandas/python/sedonadb_geopandas/_series.py: ########## @@ -0,0 +1,109 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""pandas/GeoPandas-style Series backed by a SedonaDB expression.""" + + +def _operand(other): + """Unwrap a Series to its expression; pass scalars through unchanged.""" + return other._expr if isinstance(other, Series) else other + + +class Series: + """A single column of a lazy SedonaDB frame, in the shape of a pandas Series. + + A ``Series`` pairs a source SedonaDB ``DataFrame`` with an expression over + its columns. Comparisons produce boolean ``Series`` usable as a filter mask + (``gdf[gdf["pop"] > 1000]``). Nothing is computed until ``to_pandas()`` / + display. + """ + + def __init__(self, df, expr, name): + self._df = df + self._expr = expr + self._name = name Review Comment: Noted — keeping it minimal for now: a source frame plus an expression, with comparisons and `& | ~` composition so it can act as a filter mask. Reductions and the rest of the pandas `Series` surface are deferred to a later slice, where the `sedona.spark.geopandas` `GeoSeries` is a useful reference for which parts actually get used. ########## python/sedonadb-geopandas/python/sedonadb_geopandas/_frame.py: ########## @@ -0,0 +1,90 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""GeoPandas-style GeoDataFrame backed by a lazy SedonaDB frame.""" + +from sedonadb_geopandas._series import GeoSeries, Series + + +class GeoDataFrame: + """A lazy SedonaDB frame in the shape of a ``geopandas.GeoDataFrame``. + + Wraps a SedonaDB ``DataFrame`` and tracks the active geometry column. + Row selection, column access, and geometry operations mirror GeoPandas but + build a query rather than computing eagerly; call ``to_geopandas()`` to + materialize. + """ + + def __init__(self, df, geometry="geometry"): + self._df = df + self._geometry_name = geometry + + @property + def geometry(self): + """The active geometry column as a :class:`GeoSeries`.""" + return GeoSeries(self._df, self._df[self._geometry_name], self._geometry_name) + + @property + def crs(self): + """The CRS of the active geometry column (via a zero-row materialization).""" + return self._df.limit(0).to_pandas().crs + + @property + def columns(self): + """Column names, mirroring ``GeoDataFrame.columns``.""" + return list(self._df.schema.names) + + def __getitem__(self, key): + # Boolean mask -> row filter (gdf[gdf["pop"] > 1000]). + if isinstance(key, Series): + return GeoDataFrame(self._df.filter(key._expr), self._geometry_name) + + # Column subset -> GeoDataFrame (keeps the geometry column if selected). + if isinstance(key, list): + return GeoDataFrame(self._df.select(*key), self._geometry_name) + + # Single column -> (Geo)Series. + if isinstance(key, str): + expr = self._df[key] + if key == self._geometry_name: + return GeoSeries(self._df, expr, key) + return Series(self._df, expr, key) + + raise TypeError( + f"GeoDataFrame indices must be a column name, list of names, or " + f"boolean mask, not {type(key).__name__}" + ) + + def to_crs(self, crs): + """Reproject the geometry column to ``crs`` (``ST_Transform``).""" + from sedonadb.expr import lit + + transformed = self._df[self._geometry_name].geo.transform(lit(str(crs))) Review Comment: Applied, thanks. Confirmed `lit` handles a plain string, a `pyproj.CRS` object, and an EPSG int, so the `str()` was both unnecessary and lossy for CRS objects. ########## python/sedonadb-geopandas/python/sedonadb_geopandas/_frame.py: ########## @@ -0,0 +1,90 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""GeoPandas-style GeoDataFrame backed by a lazy SedonaDB frame.""" + +from sedonadb_geopandas._series import GeoSeries, Series + + +class GeoDataFrame: + """A lazy SedonaDB frame in the shape of a ``geopandas.GeoDataFrame``. + + Wraps a SedonaDB ``DataFrame`` and tracks the active geometry column. + Row selection, column access, and geometry operations mirror GeoPandas but + build a query rather than computing eagerly; call ``to_geopandas()`` to + materialize. + """ + + def __init__(self, df, geometry="geometry"): + self._df = df + self._geometry_name = geometry + + @property + def geometry(self): + """The active geometry column as a :class:`GeoSeries`.""" + return GeoSeries(self._df, self._df[self._geometry_name], self._geometry_name) + + @property + def crs(self): + """The CRS of the active geometry column (via a zero-row materialization).""" + return self._df.limit(0).to_pandas().crs Review Comment: Applied — `.crs` is now `self._df.schema.field(self._geometry_name).type.crs`, so it no longer executes anything. Returns `None` when the frame has no active geometry column. ########## python/sedonadb-geopandas/python/sedonadb_geopandas/_frame.py: ########## @@ -0,0 +1,90 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""GeoPandas-style GeoDataFrame backed by a lazy SedonaDB frame.""" + +from sedonadb_geopandas._series import GeoSeries, Series + + +class GeoDataFrame: + """A lazy SedonaDB frame in the shape of a ``geopandas.GeoDataFrame``. + + Wraps a SedonaDB ``DataFrame`` and tracks the active geometry column. + Row selection, column access, and geometry operations mirror GeoPandas but + build a query rather than computing eagerly; call ``to_geopandas()`` to + materialize. + """ + + def __init__(self, df, geometry="geometry"): + self._df = df + self._geometry_name = geometry Review Comment: All three applied: - The default is now the internal heuristic (`primary_geometry_column()`), the same one `to_geopandas()` uses, rather than hard-coding `"geometry"`. - A non-default value is validated against the schema geometry columns: `KeyError` if the column does not exist, `ValueError` if it exists but is not a geometry. Geography is left permitted, as suggested. - `from_geopandas` now routes through `default_context().create_data_frame(...)`, so anything not dataframe-able surfaces that error. ########## python/sedonadb-geopandas/python/sedonadb_geopandas/_series.py: ########## @@ -0,0 +1,109 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""pandas/GeoPandas-style Series backed by a SedonaDB expression.""" + + +def _operand(other): + """Unwrap a Series to its expression; pass scalars through unchanged.""" + return other._expr if isinstance(other, Series) else other + + +class Series: + """A single column of a lazy SedonaDB frame, in the shape of a pandas Series. + + A ``Series`` pairs a source SedonaDB ``DataFrame`` with an expression over + its columns. Comparisons produce boolean ``Series`` usable as a filter mask + (``gdf[gdf["pop"] > 1000]``). Nothing is computed until ``to_pandas()`` / + display. + """ + + def __init__(self, df, expr, name): + self._df = df + self._expr = expr + self._name = name + + # -- element-wise comparisons -> boolean mask -------------------------- + def __gt__(self, other): + return Series(self._df, self._expr > _operand(other), self._name) + + def __ge__(self, other): + return Series(self._df, self._expr >= _operand(other), self._name) + + def __lt__(self, other): + return Series(self._df, self._expr < _operand(other), self._name) + + def __le__(self, other): + return Series(self._df, self._expr <= _operand(other), self._name) + + def __eq__(self, other): + return Series(self._df, self._expr == _operand(other), self._name) + + def __ne__(self, other): + return Series(self._df, self._expr != _operand(other), self._name) + + # -- boolean composition of masks -------------------------------------- + def __and__(self, other): + return Series(self._df, self._expr & _operand(other), self._name) + + def __or__(self, other): + return Series(self._df, self._expr | _operand(other), self._name) + + def __invert__(self): + return Series(self._df, ~self._expr, self._name) + + __hash__ = None + + # -- materialization --------------------------------------------------- + def to_pandas(self): + """Execute and return this column as a pandas (or GeoPandas) Series.""" + return self._df.select(self._expr.alias(self._name)).to_pandas()[self._name] + + def __repr__(self): + return repr(self.to_pandas()) + + +class GeoSeries(Series): + """A geometry column, in the shape of a ``geopandas.GeoSeries``. + + Element-wise geometry operations (``buffer``, ``centroid``, …) return a new + ``GeoSeries``; measures (``area``, ``length``) return a numeric ``Series``. + Each delegates to the corresponding ``ST_*`` function via SedonaDB's ``.geo`` + accessor. + """ + + def buffer(self, distance): + """Buffer each geometry by ``distance`` (``ST_Buffer``).""" + return GeoSeries(self._df, self._expr.geo.buffer(distance), self._name) Review Comment: Deferred for now, but agreed on the direction. Two things I want to check first: whether assigning `__doc__` on a property (rather than the underlying function) survives IDE hover and `help()`, and how the `.geo` text reads out of context where the wrapper signature differs (for example `area` is a property here but a function on the accessor). Worth doing as a follow-up so the markdown docs stay the single source. ########## python/sedonadb-geopandas/python/sedonadb_geopandas/__init__.py: ########## @@ -0,0 +1,51 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""GeoPandas-compatible API on top of SedonaDB. + +This package provides :class:`GeoDataFrame` / :class:`GeoSeries` wrappers whose +methods mirror GeoPandas but delegate to a lazy SedonaDB engine. It is a +compatibility layer, not a drop-in replacement: see the package README for the +intentional differences (laziness, no row index, immutability). +""" Review Comment: Added — on the module docstring, `from_geopandas`, and the `GeoDataFrame` / `Series` / `GeoSeries` class docstrings, each noting the API may change without notice. ########## python/sedonadb-geopandas/tests/test_geopandas_compat.py: ########## @@ -0,0 +1,97 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +import geopandas as gpd +import pytest + +import sedonadb_geopandas as sgpd +from sedonadb_geopandas import GeoDataFrame, GeoSeries, Series + + [email protected] +def cities(): + return gpd.GeoDataFrame( + {"name": ["A", "B", "C"], "pop": [100, 200, 300]}, + geometry=gpd.GeoSeries.from_wkt(["POINT (0 0)", "POINT (1 1)", "POINT (5 5)"]), + crs="EPSG:4326", + ) + + Review Comment: Added, close to your sketch: ```python def assert_geopandas_expr_equal(gdf, op, *, sort_by): expected = op(gdf).sort_values(sort_by).reset_index(drop=True) got = op(sgpd.from_geopandas(gdf)).to_geopandas().sort_values(sort_by).reset_index(drop=True) assert_geodataframe_equal(got, expected, check_like=True, check_crs=False) ``` Sorting and resetting the index is needed because neither row order nor the index is preserved, and `check_crs=False` tolerates the EPSG:4326 to CRS84 normalization. Using it for the filter test now; agreed it earns its keep once there are more components to throw a corpus at. ########## python/sedonadb-geopandas/python/sedonadb_geopandas/_series.py: ########## @@ -0,0 +1,109 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""pandas/GeoPandas-style Series backed by a SedonaDB expression.""" + + +def _operand(other): + """Unwrap a Series to its expression; pass scalars through unchanged.""" + return other._expr if isinstance(other, Series) else other Review Comment: Both handled. `_operand` now passes a SedonaDB `Expr` through unchanged, and rejects anything array-like (checked via `__array__`) up front with a clear message: there is no row alignment, so it suggests either staying within the frame or collecting with `to_pandas()` first. A test covers comparing against a real pandas `Series`. -- 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]
