Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Silence NumPy 1.21 DeprecationWarning by raising AttributeError #1174

Merged
merged 2 commits into from
Aug 21, 2021

Conversation

mwtoews
Copy link
Member

@mwtoews mwtoews commented Jul 20, 2021

Possible fix for NumPy 1.21 DeprecationWarning for exceptions raised in __array_interface__. Only BaseGeometry.__array_interface__ still raise NotImplementedError, as it is an abstract class.

Tests for each geometry type (except GeometryCollection) are added, which creates/assigns an object array.

Comments/reviews are welcome!

Closes #1173

@mwtoews mwtoews added this to the 1.8 milestone Jul 20, 2021
@coveralls
Copy link

coveralls commented Jul 20, 2021

Coverage Status

Coverage increased (+0.07%) to 85.352% when pulling 467fe2e on mwtoews:numpy-array-interface-dep into a228836 on Toblerity:master.

def test_numpy_empty_linearring_coords():
np = pytest.importorskip("numpy")

ring = LinearRing()
assert np.asarray(ring.coords).shape == (0,)


@pytest.mark.filterwarnings("error:An exception was ignored") # NumPy 1.21
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only test that didn't need @shapely20_deprecated to silence "ShapelyDeprecationWarning: The array interface is deprecated...", and I have no idea why. @jorisvandenbossche ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I think this is because Polygon already didn't have an array interface before (its rings have one), so for polygon the array interface didn't need to be deprecated. In addition, it also doesn't have deprecation of __len__ or iteration like the multi-part geometries (that also didn't work before), where those warnings are also typically raises when interacting with numpy.

@mwtoews mwtoews requested a review from sgillies August 20, 2021 18:06
@mwtoews
Copy link
Member Author

mwtoews commented Aug 20, 2021

@sgillies I'm keen to see this in the next pre-release

@jorisvandenbossche
Copy link
Member

@mwtoews sorry for the slow response here. I'll try to take a look tomorrow

Copy link
Contributor

@sgillies sgillies left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @mwtoews !

Copy link
Member

@jorisvandenbossche jorisvandenbossche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks good! Changing the error we raise is indeed the good solution for 1.8

I added a small change: I added a similar test for GeometryCollection (it should also work for that type, and the test passes), and also added an assert that the geometry stored in the array is correct (that it's eg not unpacked).

@@ -87,6 +87,7 @@ def test_from_numpy():
assert line.coords[:] == [(1.0, 2.0), (3.0, 4.0)]


@pytest.mark.filterwarnings("error:An exception was ignored") # NumPy 1.21
def test_numpy_empty_linestring_coords():
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This case shouldn't raise a warning? (it's directly accessing the array interface of .coords, which should work fine, also for an empty geometry giving an empty array)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops, sorry, missed the "error" :) (I thought it was ignoring those warnings)

def test_numpy_empty_linearring_coords():
np = pytest.importorskip("numpy")

ring = LinearRing()
assert np.asarray(ring.coords).shape == (0,)


@pytest.mark.filterwarnings("error:An exception was ignored") # NumPy 1.21
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I think this is because Polygon already didn't have an array interface before (its rings have one), so for polygon the array interface didn't need to be deprecated. In addition, it also doesn't have deprecation of __len__ or iteration like the multi-part geometries (that also didn't work before), where those warnings are also typically raises when interacting with numpy.

@mwtoews
Copy link
Member Author

mwtoews commented Aug 21, 2021

Thanks for the additions @jorisvandenbossche ! And also nice to see you work on a general solution for geopandas too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NumPy 1.21 __array_interface__ behavior
4 participants