Warn about deprecation of sindex.intersection objects parameter

[adriangb]

As discussed in #1439, I think it would be best to deprecate the objects parameter. This makes a warning for using it. Hopefully this will be a good first step in that direction.

[martinfleis]

Can we add tests that it properly raised a warning? Wrapping this line in this would do the job.

with pytest.warns(FutureWarning, match="A future version of GeoPandas will deprecate"):

geopandas/geopandas/tests/test_sindex.py

Line 108 in 3cba9b7

hits = list(self.df.sindex.intersection((2.5, 2.5, 4, 4), objects=True))

[adriangb]

@martinfleis are we going to make a dev branch to work on stuff that isn't intended for 0.8.0?

[brendan-ward]

How about

'objects' is deprecated and will be replaced by a different interface to labels in the sindex geometries in an upcoming release.

I don't know that the "If you use this option" is necessary here; this doesn't get added to other deprecation notices.

[adriangb]

My thoughts were that we do not really know who uses this interface and why. I thought it would be good to get some issue reports that might help shape what that future implementation of loc/label based indexing will look like, or if it is needed at all.

[jorisvandenbossche]

I think we can just say it will be removed in the future. But it might be good to indicate what to do instead (how to achieve the same behaviour without using the keyword), so it's clear for users how to replace their usage of it.

[adriangb]

So something like:

`objects` is deprecated and will be removed in a future version. Instead, use `iloc` to index your GeoSeries/GeoDataFrame using integer indexes returned by `intersection`.

?

[jorisvandenbossche]

are we going to make a dev branch to work on stuff that isn't intended for 0.8.0?

We didn't do that yet in the past (the main releases are cut from master, only for bug fix releases, we made separate branches). I would say: if you want to work on something that is only for after 0.8.0, you can already start on it / open a PR targetting master, but it can only be merged after 0.8 is released. If it turns out to be a problem in practice, we can always reconsider our branching strategy.

[jorisvandenbossche]

I think we can just say it will be removed in the future. But it might be good to indicate what to do instead (how to achieve the same behaviour without using the keyword), so it's clear for users how to replace their usage of it.

[adriangb]

I pushed the changes to the warning/docs text

[adriangb]

Pinging you on this one as well @martinfleis, let me know if there's more discussion needed or anything I can do on my end. Thanks!

[martinfleis]

Thank you @adriangb!