You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@sedona.apache.org by "petern48 (via GitHub)" <gi...@apache.org> on 2025/07/02 16:16:52 UTC
[PR] [GH-2045] Geopandas.GeoSeries: Implement fillna [sedona]
petern48 opened a new pull request, #2046:
URL: https://github.com/apache/sedona/pull/2046
## Did you read the Contributor Guide?
- Yes, I have read the [Contributor Rules](https://sedona.apache.org/latest/community/rule/) and [Contributor Development Guide](https://sedona.apache.org/latest/community/develop/)
## Is this PR related to a ticket?
- Yes, and the PR name follows the format `[GH-XXX] my subject`. Closes #2045
## What changes were proposed in this PR?
Implement fillna
## How was this patch tested?
Added tests
## Did this PR include necessary documentation updates?
- Yes, I have updated the documentation.
--
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: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [GH-2045] Geopandas.GeoSeries: Implement fillna [sedona]
Posted by "jiayuasu (via GitHub)" <gi...@apache.org>.
jiayuasu merged PR #2046:
URL: https://github.com/apache/sedona/pull/2046
--
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: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [GH-2045] Geopandas.GeoSeries: Implement fillna [sedona]
Posted by "petern48 (via GitHub)" <gi...@apache.org>.
petern48 commented on PR #2046:
URL: https://github.com/apache/sedona/pull/2046#issuecomment-3063260712
Tests are now passing again after merging in the recent changes
--
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: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [GH-2045] Geopandas.GeoSeries: Implement fillna [sedona]
Posted by "zhangfengcdt (via GitHub)" <gi...@apache.org>.
zhangfengcdt commented on code in PR #2046:
URL: https://github.com/apache/sedona/pull/2046#discussion_r2202059319
##########
python/sedona/geopandas/geoseries.py:
##########
@@ -537,7 +537,9 @@ def _to_geopandas(self) -> gpd.GeoSeries:
try:
return gpd.GeoSeries(
pd_series.map(
- lambda wkb: shapely.wkb.loads(bytes(wkb)) if wkb else None
+ lambda wkb: (
+ shapely.wkb.loads(bytes(wkb)) if not pd.isna(wkb) else None
Review Comment:
I think the issue is that if user get that error, e.g., WKBReadingError because of empty byte strings are not valid WKB representations, they won't be able to know what to do and where that wkb come from - unless if they pass that as a parameter explicitly. We would either:
- add a document to tell user what the error WKBReadingError meant to them or
- process / skip them as NA values
Either way, this should be documented in the public method, i.e., to_geopandas
--
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: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [GH-2045] Geopandas.GeoSeries: Implement fillna [sedona]
Posted by "zhangfengcdt (via GitHub)" <gi...@apache.org>.
zhangfengcdt commented on code in PR #2046:
URL: https://github.com/apache/sedona/pull/2046#discussion_r2188440761
##########
python/sedona/geopandas/geoseries.py:
##########
@@ -1374,8 +1374,128 @@ def notnull(self) -> pspd.Series:
"""Alias for `notna` method. See `notna` for more detail."""
return self.notna()
- def fillna(self, value: Any) -> "GeoSeries":
- raise NotImplementedError("GeoSeries.fillna() is not implemented yet.")
+ def fillna(
+ self, value=None, inplace: bool = False, limit=None, **kwargs
+ ) -> Union["GeoSeries", None]:
+ """
+ Fill NA values with geometry (or geometries).
+
+ Parameters
+ ----------
+ value : shapely geometry or GeoSeries, default None
+ If None is passed, NA values will be filled with GEOMETRYCOLLECTION EMPTY.
+ If a shapely geometry object is passed, it will be
+ used to fill all missing values. If a ``GeoSeries`` or ``GeometryArray``
+ are passed, missing values will be filled based on the corresponding index
+ locations. If pd.NA or np.nan are passed, values will be filled with
+ ``None`` (not GEOMETRYCOLLECTION EMPTY).
+ limit : int, default None
+ This is the maximum number of entries along the entire axis
+ where NaNs will be filled. Must be greater than 0 if not None.
+
+ Returns
+ -------
+ GeoSeries
+
+ Examples
+ --------
+
+ >>> from sedona.geopandas import GeoSeries
+ >>> from shapely.geometry import Polygon
+ >>> s = GeoSeries(
+ ... [
+ ... Polygon([(0, 0), (1, 1), (0, 1)]),
+ ... None,
+ ... Polygon([(0, 0), (-1, 1), (0, -1)]),
+ ... ]
+ ... )
+ >>> s
+ 0 POLYGON ((0 0, 1 1, 0 1, 0 0))
+ 1 None
+ 2 POLYGON ((0 0, -1 1, 0 -1, 0 0))
+ dtype: geometry
+
+ Filled with an empty polygon.
+
+ >>> s.fillna()
+ 0 POLYGON ((0 0, 1 1, 0 1, 0 0))
+ 1 GEOMETRYCOLLECTION EMPTY
+ 2 POLYGON ((0 0, -1 1, 0 -1, 0 0))
+ dtype: geometry
+
+ Filled with a specific polygon.
+
+ >>> s.fillna(Polygon([(0, 1), (2, 1), (1, 2)]))
+ 0 POLYGON ((0 0, 1 1, 0 1, 0 0))
+ 1 POLYGON ((0 1, 2 1, 1 2, 0 1))
+ 2 POLYGON ((0 0, -1 1, 0 -1, 0 0))
+ dtype: geometry
+
+ Filled with another GeoSeries.
+
+ >>> from shapely.geometry import Point
+ >>> s_fill = GeoSeries(
+ ... [
+ ... Point(0, 0),
+ ... Point(1, 1),
+ ... Point(2, 2),
+ ... ]
+ ... )
+ >>> s.fillna(s_fill)
+ 0 POLYGON ((0 0, 1 1, 0 1, 0 0))
+ 1 POINT (1 1)
+ 2 POLYGON ((0 0, -1 1, 0 -1, 0 0))
+ dtype: geometry
+
+ See Also
+ --------
+ GeoSeries.isna : detect missing values
+ """
+ from shapely.geometry.base import BaseGeometry
+ from geopandas.array import GeometryArray
+
+ # TODO: Implement limit
+ if limit:
+ raise NotImplementedError(
+ "GeoSeries.fillna() with limit is not implemented yet."
+ )
+
+ col = self.get_first_geometry_column()
+ if pd.isna(value) == True or isinstance(value, BaseGeometry):
+ if (
+ value is not None and pd.isna(value) == True
+ ): # ie. value is np.nan or pd.NA:
+ value = "NULL"
+ else:
+ if value is None:
+ from shapely.geometry import GeometryCollection
+
+ value = GeometryCollection()
+
+ value = f"ST_GeomFromText('{value.wkt}')"
+
+ select = f"COALESCE(`{col}`, {value})"
+ result = self._query_geometry_column(select, col, "")
+ elif isinstance(value, (GeoSeries, GeometryArray, gpd.GeoSeries)):
+
+ if isinstance(value, (gpd.GeoSeries, GeometryArray)):
+ value = GeoSeries(value)
+
+ # Replace all None's with empty geometries
+ value = value.fillna(None)
+
+ select = f"COALESCE(`L`, `R`)"
Review Comment:
Can we comment on why using coalesce here?
##########
python/sedona/geopandas/geoseries.py:
##########
@@ -1374,8 +1374,128 @@ def notnull(self) -> pspd.Series:
"""Alias for `notna` method. See `notna` for more detail."""
return self.notna()
- def fillna(self, value: Any) -> "GeoSeries":
- raise NotImplementedError("GeoSeries.fillna() is not implemented yet.")
+ def fillna(
+ self, value=None, inplace: bool = False, limit=None, **kwargs
+ ) -> Union["GeoSeries", None]:
+ """
+ Fill NA values with geometry (or geometries).
+
+ Parameters
+ ----------
+ value : shapely geometry or GeoSeries, default None
+ If None is passed, NA values will be filled with GEOMETRYCOLLECTION EMPTY.
+ If a shapely geometry object is passed, it will be
+ used to fill all missing values. If a ``GeoSeries`` or ``GeometryArray``
+ are passed, missing values will be filled based on the corresponding index
+ locations. If pd.NA or np.nan are passed, values will be filled with
+ ``None`` (not GEOMETRYCOLLECTION EMPTY).
+ limit : int, default None
+ This is the maximum number of entries along the entire axis
+ where NaNs will be filled. Must be greater than 0 if not None.
+
+ Returns
+ -------
+ GeoSeries
+
+ Examples
+ --------
+
+ >>> from sedona.geopandas import GeoSeries
+ >>> from shapely.geometry import Polygon
+ >>> s = GeoSeries(
+ ... [
+ ... Polygon([(0, 0), (1, 1), (0, 1)]),
+ ... None,
+ ... Polygon([(0, 0), (-1, 1), (0, -1)]),
+ ... ]
+ ... )
+ >>> s
+ 0 POLYGON ((0 0, 1 1, 0 1, 0 0))
+ 1 None
+ 2 POLYGON ((0 0, -1 1, 0 -1, 0 0))
+ dtype: geometry
+
+ Filled with an empty polygon.
+
+ >>> s.fillna()
+ 0 POLYGON ((0 0, 1 1, 0 1, 0 0))
+ 1 GEOMETRYCOLLECTION EMPTY
+ 2 POLYGON ((0 0, -1 1, 0 -1, 0 0))
+ dtype: geometry
+
+ Filled with a specific polygon.
+
+ >>> s.fillna(Polygon([(0, 1), (2, 1), (1, 2)]))
+ 0 POLYGON ((0 0, 1 1, 0 1, 0 0))
+ 1 POLYGON ((0 1, 2 1, 1 2, 0 1))
+ 2 POLYGON ((0 0, -1 1, 0 -1, 0 0))
+ dtype: geometry
+
+ Filled with another GeoSeries.
+
+ >>> from shapely.geometry import Point
+ >>> s_fill = GeoSeries(
+ ... [
+ ... Point(0, 0),
+ ... Point(1, 1),
+ ... Point(2, 2),
+ ... ]
+ ... )
+ >>> s.fillna(s_fill)
+ 0 POLYGON ((0 0, 1 1, 0 1, 0 0))
+ 1 POINT (1 1)
+ 2 POLYGON ((0 0, -1 1, 0 -1, 0 0))
+ dtype: geometry
+
+ See Also
+ --------
+ GeoSeries.isna : detect missing values
+ """
+ from shapely.geometry.base import BaseGeometry
+ from geopandas.array import GeometryArray
+
+ # TODO: Implement limit
Review Comment:
Can you create a github issue and link the todo to that issue?
##########
python/sedona/geopandas/geoseries.py:
##########
@@ -1374,8 +1374,128 @@ def notnull(self) -> pspd.Series:
"""Alias for `notna` method. See `notna` for more detail."""
return self.notna()
- def fillna(self, value: Any) -> "GeoSeries":
- raise NotImplementedError("GeoSeries.fillna() is not implemented yet.")
+ def fillna(
+ self, value=None, inplace: bool = False, limit=None, **kwargs
+ ) -> Union["GeoSeries", None]:
+ """
+ Fill NA values with geometry (or geometries).
+
+ Parameters
+ ----------
+ value : shapely geometry or GeoSeries, default None
+ If None is passed, NA values will be filled with GEOMETRYCOLLECTION EMPTY.
+ If a shapely geometry object is passed, it will be
+ used to fill all missing values. If a ``GeoSeries`` or ``GeometryArray``
+ are passed, missing values will be filled based on the corresponding index
+ locations. If pd.NA or np.nan are passed, values will be filled with
+ ``None`` (not GEOMETRYCOLLECTION EMPTY).
+ limit : int, default None
+ This is the maximum number of entries along the entire axis
+ where NaNs will be filled. Must be greater than 0 if not None.
+
+ Returns
+ -------
+ GeoSeries
+
+ Examples
+ --------
+
+ >>> from sedona.geopandas import GeoSeries
+ >>> from shapely.geometry import Polygon
+ >>> s = GeoSeries(
+ ... [
+ ... Polygon([(0, 0), (1, 1), (0, 1)]),
+ ... None,
+ ... Polygon([(0, 0), (-1, 1), (0, -1)]),
+ ... ]
+ ... )
+ >>> s
+ 0 POLYGON ((0 0, 1 1, 0 1, 0 0))
+ 1 None
+ 2 POLYGON ((0 0, -1 1, 0 -1, 0 0))
+ dtype: geometry
+
+ Filled with an empty polygon.
+
+ >>> s.fillna()
+ 0 POLYGON ((0 0, 1 1, 0 1, 0 0))
+ 1 GEOMETRYCOLLECTION EMPTY
+ 2 POLYGON ((0 0, -1 1, 0 -1, 0 0))
+ dtype: geometry
+
+ Filled with a specific polygon.
+
+ >>> s.fillna(Polygon([(0, 1), (2, 1), (1, 2)]))
+ 0 POLYGON ((0 0, 1 1, 0 1, 0 0))
+ 1 POLYGON ((0 1, 2 1, 1 2, 0 1))
+ 2 POLYGON ((0 0, -1 1, 0 -1, 0 0))
+ dtype: geometry
+
+ Filled with another GeoSeries.
+
+ >>> from shapely.geometry import Point
+ >>> s_fill = GeoSeries(
+ ... [
+ ... Point(0, 0),
+ ... Point(1, 1),
+ ... Point(2, 2),
+ ... ]
+ ... )
+ >>> s.fillna(s_fill)
+ 0 POLYGON ((0 0, 1 1, 0 1, 0 0))
+ 1 POINT (1 1)
+ 2 POLYGON ((0 0, -1 1, 0 -1, 0 0))
+ dtype: geometry
+
+ See Also
+ --------
+ GeoSeries.isna : detect missing values
+ """
+ from shapely.geometry.base import BaseGeometry
+ from geopandas.array import GeometryArray
+
+ # TODO: Implement limit
+ if limit:
+ raise NotImplementedError(
+ "GeoSeries.fillna() with limit is not implemented yet."
+ )
+
+ col = self.get_first_geometry_column()
+ if pd.isna(value) == True or isinstance(value, BaseGeometry):
+ if (
+ value is not None and pd.isna(value) == True
+ ): # ie. value is np.nan or pd.NA:
+ value = "NULL"
+ else:
+ if value is None:
+ from shapely.geometry import GeometryCollection
+
+ value = GeometryCollection()
Review Comment:
Why do we use GeometryCollection here?
--
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: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [GH-2045] Geopandas.GeoSeries: Implement fillna [sedona]
Posted by "petern48 (via GitHub)" <gi...@apache.org>.
petern48 commented on code in PR #2046:
URL: https://github.com/apache/sedona/pull/2046#discussion_r2202090738
##########
python/sedona/geopandas/geoseries.py:
##########
@@ -537,7 +537,9 @@ def _to_geopandas(self) -> gpd.GeoSeries:
try:
return gpd.GeoSeries(
pd_series.map(
- lambda wkb: shapely.wkb.loads(bytes(wkb)) if wkb else None
+ lambda wkb: (
+ shapely.wkb.loads(bytes(wkb)) if not pd.isna(wkb) else None
Review Comment:
I agree that it would be nice to tell the user what the error indicates, but as I mentioned, this isn't expected behavior. In the case where an empty byte string is returned, I think this would be our error (the Sedona developer's error) and not the user's error, so I don't know what to tell them. Do you know what might cause an empty byte string to be returned here? What would you recommend specifying in the docstring?
--
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: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [GH-2045] Geopandas.GeoSeries: Implement fillna [sedona]
Posted by "petern48 (via GitHub)" <gi...@apache.org>.
petern48 commented on code in PR #2046:
URL: https://github.com/apache/sedona/pull/2046#discussion_r2190588062
##########
python/sedona/geopandas/geoseries.py:
##########
@@ -1374,8 +1374,128 @@ def notnull(self) -> pspd.Series:
"""Alias for `notna` method. See `notna` for more detail."""
return self.notna()
- def fillna(self, value: Any) -> "GeoSeries":
- raise NotImplementedError("GeoSeries.fillna() is not implemented yet.")
+ def fillna(
+ self, value=None, inplace: bool = False, limit=None, **kwargs
+ ) -> Union["GeoSeries", None]:
+ """
+ Fill NA values with geometry (or geometries).
+
+ Parameters
+ ----------
+ value : shapely geometry or GeoSeries, default None
+ If None is passed, NA values will be filled with GEOMETRYCOLLECTION EMPTY.
+ If a shapely geometry object is passed, it will be
+ used to fill all missing values. If a ``GeoSeries`` or ``GeometryArray``
+ are passed, missing values will be filled based on the corresponding index
+ locations. If pd.NA or np.nan are passed, values will be filled with
+ ``None`` (not GEOMETRYCOLLECTION EMPTY).
+ limit : int, default None
+ This is the maximum number of entries along the entire axis
+ where NaNs will be filled. Must be greater than 0 if not None.
+
+ Returns
+ -------
+ GeoSeries
+
+ Examples
+ --------
+
+ >>> from sedona.geopandas import GeoSeries
+ >>> from shapely.geometry import Polygon
+ >>> s = GeoSeries(
+ ... [
+ ... Polygon([(0, 0), (1, 1), (0, 1)]),
+ ... None,
+ ... Polygon([(0, 0), (-1, 1), (0, -1)]),
+ ... ]
+ ... )
+ >>> s
+ 0 POLYGON ((0 0, 1 1, 0 1, 0 0))
+ 1 None
+ 2 POLYGON ((0 0, -1 1, 0 -1, 0 0))
+ dtype: geometry
+
+ Filled with an empty polygon.
+
+ >>> s.fillna()
+ 0 POLYGON ((0 0, 1 1, 0 1, 0 0))
+ 1 GEOMETRYCOLLECTION EMPTY
+ 2 POLYGON ((0 0, -1 1, 0 -1, 0 0))
+ dtype: geometry
+
+ Filled with a specific polygon.
+
+ >>> s.fillna(Polygon([(0, 1), (2, 1), (1, 2)]))
+ 0 POLYGON ((0 0, 1 1, 0 1, 0 0))
+ 1 POLYGON ((0 1, 2 1, 1 2, 0 1))
+ 2 POLYGON ((0 0, -1 1, 0 -1, 0 0))
+ dtype: geometry
+
+ Filled with another GeoSeries.
+
+ >>> from shapely.geometry import Point
+ >>> s_fill = GeoSeries(
+ ... [
+ ... Point(0, 0),
+ ... Point(1, 1),
+ ... Point(2, 2),
+ ... ]
+ ... )
+ >>> s.fillna(s_fill)
+ 0 POLYGON ((0 0, 1 1, 0 1, 0 0))
+ 1 POINT (1 1)
+ 2 POLYGON ((0 0, -1 1, 0 -1, 0 0))
+ dtype: geometry
+
+ See Also
+ --------
+ GeoSeries.isna : detect missing values
+ """
+ from shapely.geometry.base import BaseGeometry
+ from geopandas.array import GeometryArray
+
+ # TODO: Implement limit
+ if limit:
+ raise NotImplementedError(
+ "GeoSeries.fillna() with limit is not implemented yet."
+ )
+
+ col = self.get_first_geometry_column()
+ if pd.isna(value) == True or isinstance(value, BaseGeometry):
+ if (
+ value is not None and pd.isna(value) == True
+ ): # ie. value is np.nan or pd.NA:
+ value = "NULL"
+ else:
+ if value is None:
+ from shapely.geometry import GeometryCollection
+
+ value = GeometryCollection()
Review Comment:
"If None is passed, NA values will be filled with GEOMETRYCOLLECTION EMPTY." This is already in the docstring, so I don't see the need for an additional comment
--
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: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [GH-2045] Geopandas.GeoSeries: Implement fillna [sedona]
Posted by "petern48 (via GitHub)" <gi...@apache.org>.
petern48 commented on code in PR #2046:
URL: https://github.com/apache/sedona/pull/2046#discussion_r2202048320
##########
python/sedona/geopandas/geoseries.py:
##########
@@ -537,7 +537,9 @@ def _to_geopandas(self) -> gpd.GeoSeries:
try:
return gpd.GeoSeries(
pd_series.map(
- lambda wkb: shapely.wkb.loads(bytes(wkb)) if wkb else None
+ lambda wkb: (
+ shapely.wkb.loads(bytes(wkb)) if not pd.isna(wkb) else None
Review Comment:
Empty byte strings are unexpected behavior, so I'd rather not check for them here and let them error in the event that they occur.
--
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: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [GH-2045] Geopandas.GeoSeries: Implement fillna [sedona]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #2046:
URL: https://github.com/apache/sedona/pull/2046#discussion_r2202031422
##########
python/sedona/geopandas/geoseries.py:
##########
@@ -537,7 +537,9 @@ def _to_geopandas(self) -> gpd.GeoSeries:
try:
return gpd.GeoSeries(
pd_series.map(
- lambda wkb: shapely.wkb.loads(bytes(wkb)) if wkb else None
+ lambda wkb: (
+ shapely.wkb.loads(bytes(wkb)) if not pd.isna(wkb) else None
Review Comment:
The current check only skips NaN/NA but will attempt to load empty byte strings, which may cause a WKB parse error. Add a truthiness check (e.g. `if wkb and not pd.isna(wkb)`) to skip empty bytes.
```suggestion
shapely.wkb.loads(bytes(wkb)) if wkb and not pd.isna(wkb) else None
```
--
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: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org