DEP: GeoSeries.geom_almost_equals #2604

darshanip · 2022-10-18T19:07:03Z

Need confirmation, apart from the deletion I did do we need to remove 'geom_almost_equals' and its usages as well?
This is first code contribution in OS.

darshanip · 2022-10-19T16:53:04Z

hi @m-richards , could you please check on this.

m-richards · 2022-10-22T02:33:52Z

Hi @darshanip, this is unused code so is good to delete. With geom_almost_equals, we won't remove that yet, but should rather add a FutureWarning to indicate to users it will be removed in a future version.

The warnings removed in #2267 are an example of the kind of message you should add here if you'd like some guidance.

Due to linter error. geopandas/base.py:1100:89: E501 line too long (91 > 88 characters) geopandas/array.py:573:89: E501 line too long (91 > 88 characters)

darshanip · 2022-11-03T19:11:57Z

@m-richards Could you please review the changes.

darshanip · 2022-11-11T12:36:54Z

@jorisvandenbossche @martinfleis, Could you please review changes.

m-richards

Hi @darshanip, sorry for the delayed response.

This is looking good so far but there is a little bit more to do:

We should update the tests using geom_almost_equals to use geom_equals_exact instead and the helper method in testing.py
In test_geoseries::test_geom_almost_equals we should add a test to check the correct warning is emitted, and filter the warning from the other usages.
We should update the documentation in data_structures.rst to refer to geom_equals_exact instead.

geopandas/array.py

geopandas/base.py

- Updated shapely warning - Updated tests using geom_almost_equals to geom_equals_exact - Updated documentaion in data_structures.rst to refer geom_equals_exact instead. Pending - New test required for correct warning.

for 88 chars

darshanip · 2023-04-27T14:26:49Z

@m-richards can you please review code.

This reverts commit fd20790.

geopandas/tests/test_geoseries.py

m-richards

Hi @darshanip, I've left a handful of further comments which should help with fixing some of the test errors.

geopandas/array.py

doc/source/docs/user_guide/data_structures.rst

geopandas/base.py

geopandas/testing.py

m-richards · 2023-05-01T08:27:15Z

geopandas/tests/test_dissolve.py

@@ -61,7 +61,7 @@ def expected_mean(merged_shapes):
 def test_geom_dissolve(nybb_polydf, first):
    test = nybb_polydf.dissolve("manhattan_bronx")
    assert test.geometry.name == "myshapes"
-    assert test.geom_almost_equals(first).all()
+    assert test.geom_equals_exact(first).all()


This also needs to be updated to provide the second parameter for geom_equals_exact

geopandas/tests/test_geom_methods.py

geopandas/tests/test_geoseries.py

geopandas/base.py

Co-authored-by: Matt Richards <45483497+m-richards@users.noreply.github.com>

geopandas/testing.py

m-richards · 2023-07-23T05:57:30Z

I've revived this, should be close to ready now. I'm not sure if we should also deprecate geopandas.testing.geom_almost_equals. I don't really see a huge issue with keeping it. If we were to deprecate, it would definitely be worth keeping a private internal helper for this.

martinfleis

Thanks!

I'm not sure if we should also deprecate geopandas.testing.geom_almost_equals.

Fine with keeping it as well.

CHANGELOG.md

Co-authored-by: Martin Fleischmann <martin@martinfleischmann.net>

martinfleis

We cannot add a warning on both the array level and the base level as the warning is then emitted twice if the method is accessed from GeoSeries/DataFrame.

In [2]: from geodatasets import get_path

In [3]: df = geopandas.read_file(get_path('nybb'))

In [4]: df.geom_almost_equals(df)
<ipython-input-4-6eaa63e61711>:1: FutureWarning: The 'geom_almost_equals()' method is deprecated because the name isconfusing. The 'geom_equals_exact()' method should be used instead.
  df.geom_almost_equals(df)
/Users/martin/Git/geopandas/geopandas/base.py:47: FutureWarning: The 'geom_almost_equals()' method is deprecated because the name isconfusing. The 'geom_equals_exact()' method should be used instead.
  data = getattr(a_this, op)(other, *args, **kwargs)

We should have it one one level only, probably on array. We could also keep it on both level but use geom_equals_exact array-level method in base-level geom_almost_equals.

m-richards · 2023-09-14T10:37:29Z

We should have it one one level only, probably on array. We could also keep it on both level but use geom_equals_exact array-level method in base-level geom_almost_equals.

I've done the latter, it seems better to keep the warning in base.py so the stacklevel is correct for an end user.

m-richards · 2023-09-14T10:44:37Z

@martinfleis I've had to continue this at #3017 as I seem to no longer have push permissions on the fork.

martinfleis · 2023-09-15T07:52:00Z

Superseded by #3017

Removed almost_equals

3ff51e3

martinfleis mentioned this pull request Oct 21, 2022

Deprecate GeoSeries.almost_equals (deprecated in shapely) #2538

Closed

darshanip added 3 commits November 3, 2022 23:57

warning added

c82ec30

String length reduced.

e70cc08

Due to linter error. geopandas/base.py:1100:89: E501 line too long (91 > 88 characters) geopandas/array.py:573:89: E501 line too long (91 > 88 characters)

warn updated

1ea2219

m-richards reviewed Nov 13, 2022

View reviewed changes

geopandas/array.py Outdated Show resolved Hide resolved

geopandas/base.py Outdated Show resolved Hide resolved

darshanip and others added 5 commits November 15, 2022 01:11

resolved conflicts.

03f973f

- Updated shapely warning - Updated tests using geom_almost_equals to geom_equals_exact - Updated documentaion in data_structures.rst to refer geom_equals_exact instead. Pending - New test required for correct warning.

resolve linter error

7cff540

for 88 chars

resolve linter error

6aaa113

removed unused

864acee

Merge branch 'geopandas:main' into main

a9331b1

darshanip requested a review from m-richards April 27, 2023 14:25

darshanip and others added 3 commits April 29, 2023 20:03

Update setup.py

fd20790

Revert "Update setup.py"

df71713

This reverts commit fd20790.

clean-up mentions in docstrings

5d2b26e

martinfleis reviewed Apr 29, 2023

View reviewed changes

geopandas/tests/test_geoseries.py Outdated Show resolved Hide resolved

resolve conv

4bf7eed

darshanip requested a review from martinfleis April 30, 2023 14:29

m-richards reviewed May 1, 2023

View reviewed changes

darshanip and others added 5 commits May 2, 2023 18:25

Update geopandas/array.py

45f5fab

Co-authored-by: Matt Richards <45483497+m-richards@users.noreply.github.com>

Update doc/source/docs/user_guide/data_structures.rst

ec6f11c

Co-authored-by: Matt Richards <45483497+m-richards@users.noreply.github.com>

Update geopandas/base.py

9ea2645

Co-authored-by: Matt Richards <45483497+m-richards@users.noreply.github.com>

Update geopandas/testing.py

fc74c88

Co-authored-by: Matt Richards <45483497+m-richards@users.noreply.github.com>

Update geopandas/testing.py

dcff61c

Co-authored-by: Matt Richards <45483497+m-richards@users.noreply.github.com>

martinfleis reviewed May 2, 2023

View reviewed changes

geopandas/testing.py Outdated Show resolved Hide resolved

martinfleis and others added 6 commits May 2, 2023 16:36

Update geopandas/testing.py

731c321

update to resolve issues

6f60895

Merge remote-tracking branch 'upstream/main' into darshanip/main

5a50a97

add changelog note

920ae29

cln

b034124

fix doctest

eb43b4a

m-richards changed the title ~~DEP: GeoSeries.almost_equals~~ DEP: GeoSeries.geom_almost_equals Jul 23, 2023

martinfleis approved these changes Jul 26, 2023

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

martinfleis added this to the 0.14 milestone Jul 26, 2023

m-richards and others added 2 commits August 9, 2023 22:05

Update CHANGELOG.md

8acfebe

Co-authored-by: Martin Fleischmann <martin@martinfleischmann.net>

Merge remote-tracking branch 'upstream/main' into pr/darshanip/2604

3ed9246

martinfleis requested changes Sep 13, 2023

View reviewed changes

martinfleis modified the milestones: 0.14, 1.0 Sep 13, 2023

m-richards mentioned this pull request Sep 14, 2023

DEP: GeoSeries.geom_almost_equals #3017

Merged

martinfleis closed this Sep 15, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DEP: GeoSeries.geom_almost_equals #2604

DEP: GeoSeries.geom_almost_equals #2604

darshanip commented Oct 18, 2022 •

edited

Loading

darshanip commented Oct 19, 2022

m-richards commented Oct 22, 2022

darshanip commented Nov 3, 2022 •

edited

Loading

darshanip commented Nov 11, 2022

m-richards left a comment

darshanip commented Apr 27, 2023

m-richards left a comment

m-richards May 1, 2023

m-richards commented Jul 23, 2023

martinfleis left a comment

martinfleis left a comment

m-richards commented Sep 14, 2023

m-richards commented Sep 14, 2023

martinfleis commented Sep 15, 2023

DEP: GeoSeries.geom_almost_equals #2604

DEP: GeoSeries.geom_almost_equals #2604

Conversation

darshanip commented Oct 18, 2022 • edited Loading

darshanip commented Oct 19, 2022

m-richards commented Oct 22, 2022

darshanip commented Nov 3, 2022 • edited Loading

darshanip commented Nov 11, 2022

m-richards left a comment

Choose a reason for hiding this comment

darshanip commented Apr 27, 2023

m-richards left a comment

Choose a reason for hiding this comment

m-richards May 1, 2023

Choose a reason for hiding this comment

m-richards commented Jul 23, 2023

martinfleis left a comment

Choose a reason for hiding this comment

martinfleis left a comment

Choose a reason for hiding this comment

m-richards commented Sep 14, 2023

m-richards commented Sep 14, 2023

martinfleis commented Sep 15, 2023

darshanip commented Oct 18, 2022 •

edited

Loading

darshanip commented Nov 3, 2022 •

edited

Loading