[GH-2454] : Implement binary predicate relate for Geopandas
#2455
Conversation
…esentation Resolves: apache#2454 Depends on: apache#2230
gauravbyte
changed the title
relate for Geopandasrelate for Geopandas
petern48
left a comment
petern48
left a comment
There was a problem hiding this comment.
Welcome! Thanks for working on this. It looks great. I mainly just had two minor suggestions to improve testing, but otherwise it looks good.
| point = Point(0, 0) | ||
| result = s.relate(point) | ||
| expected = pd.Series(["0FFFFFFF2", "0FFFFFFF2", "FF10F0FF2"]) | ||
| assert_series_equal(result.to_pandas(), expected) |
There was a problem hiding this comment.
| assert_series_equal(result.to_pandas(), expected) | |
| self.check_pd_series_equal(result, expected) |
While this works, it's actually preferred to use self.check_pd_series_equal() instead. You can read about why in this issue: #2379.
| ) | ||
| result = s3.relate(s4, align=True) | ||
| expected = pd.Series([None, "FF0FFF0F2", None], index=[0, 1, 2]) | ||
| assert_series_equal(result.to_pandas(), expected) |
There was a problem hiding this comment.
| assert_series_equal(result.to_pandas(), expected) | |
| self.check_pd_series_equal(result, expected) |
| # 4. Check that GeoDataFrame works too | ||
| df_result = s.to_geoframe().relate(s2, align=False) | ||
| expected = pd.Series(["0FFFFFFF2", "FF0FFF0F2", "1FFF0FFF2"]) | ||
| assert_series_equal(df_result.to_pandas(), expected) |
There was a problem hiding this comment.
| assert_series_equal(df_result.to_pandas(), expected) | |
| self.check_pd_series_equal(result, expected) |
| if self.contains_any_geom_collection(geom, geom2): | ||
| continue |
There was a problem hiding this comment.
| if self.contains_any_geom_collection(geom, geom2): | |
| continue |
This conditional skip is included in some test functions to work around a bug or difference in Sedona's behavior, but for this case, we can actually remove this, since the tests pass without it (I checked this branch out and tried it locally).
There was a problem hiding this comment.
Resolved in the commit Resolved in the commit 6f14d51
optional: I recommend setting up the |
…ation for GeoPandas Resolves: apache#2454 Depends on: apache#2230
petern48
left a comment
petern48
left a comment
There was a problem hiding this comment.
Great PR, thanks for working on it!
3 participants
Resolves: #2454
Depends on: #2230
Did you read the Contributor Guide?
Is this PR related to a ticket?
[GH-XXX] my subject. Closes Geopandas: Implement Binary predicaterelate#2454What changes were proposed in this PR?
Implement binary predicate
relatefor Geopandas with test casesHow was this patch tested?
Added tests
Did this PR include necessary documentation updates?