Skip to content

Add migration guide with explanation how to update your code for Shapely 1.8 / 2.0#1180

Merged
jorisvandenbossche merged 9 commits intoshapely:masterfrom
jorisvandenbossche:migration-guide
Sep 8, 2021
Merged

Add migration guide with explanation how to update your code for Shapely 1.8 / 2.0#1180
jorisvandenbossche merged 9 commits intoshapely:masterfrom
jorisvandenbossche:migration-guide

Conversation

@jorisvandenbossche
Copy link
Copy Markdown
Member

In preparation of a next (final?) Shapely 1.8 release, I wrote up a more detailed page on the warnings included in Shapely 1.8 (and upcoming changes in 2.0), and how to update your code to get rid of the warnings / prepare it for Shapely 2.0.


- ops.cascaded_union
- geometry .empty()
- geometry .to_wkb() and .to_wkt()
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Those were actually removed and not deprecated, so updated the note (they were deprecated long time ago, see #974)

'my_geometry'

This doesn't raise a warning in Shapely 1.8, but will start raising an
AttributeError in Shapely 2.0.
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I realized this while writing this guide, but we could actually still add a warning for this. That might be useful?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Probably a good idea, if it's simple to implement? Using __getattr__()?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Looking into it, what I wrote here is actually not completely correct. So PyGEOS objects are indeed fully immutable (a C extension type):

In [1]: import pygeos

In [2]: p = pygeos.Geometry("POINT(1 1)")

In [3]: p
Out[3]: <pygeos.Geometry POINT (1 1)>

In [4]: p.name = "test"
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-4-3cf5ac6b0436> in <module>
----> 1 p.name = "test"

AttributeError: 'pygeos.lib.Geometry' object has no attribute 'name'

But when using the shapely-2.0 branch, we actually create python subclasses for each geometry type (and register those to pygeos), and those subclasses are still mutable:

In [1]: import pygeos

In [2]: p = pygeos.Geometry("POINT(1 1)")

In [3]: p
Out[3]: <shapely.geometry.Point POINT (1 1)>

In [4]: p.name = "test"

In [5]: p.name
Out[5]: 'test'

So in the end, it's not an inevitable consequence of the refactor, but we still have the choice whether to allow or disallow this.
I made a PR #1181 to illustrate how we can disallow this in Shapely 2.0, but to discuss of course whether we want this or not.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

And also opened a PR to deprecate it for Shapely 1.8: #1183

@jorisvandenbossche
Copy link
Copy Markdown
Member Author

I should probably still add some more content about putting geometries in numpy arrays, cfr #1096

@coveralls
Copy link
Copy Markdown

coveralls commented Aug 21, 2021

Coverage Status

Coverage increased (+0.01%) to 85.364% when pulling bda69b8 on jorisvandenbossche:migration-guide into 9ff8c32 on Toblerity:master.

Copy link
Copy Markdown
Member

@mwtoews mwtoews left a comment

Choose a reason for hiding this comment

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

Documentation looks well written, and covers the main user-cases that might be encountered.

There's only one potentially missing item, concerning object arrays.

'my_geometry'

This doesn't raise a warning in Shapely 1.8, but will start raising an
AttributeError in Shapely 2.0.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Probably a good idea, if it's simple to implement? Using __getattr__()?

Shapely 2.0, but since Shapely will start requiring NumPy as a dependency,
you can use NumPy or its array interface directly.


Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Another related issue is creating an object array of shapely geometries, described in #1096. Is it worth discussing? It only impacts version 1.8.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, I certainly think that's worth describing (see also my comment #1180 (comment) above). Will add a section about that.

@jorisvandenbossche
Copy link
Copy Markdown
Member Author

@mwtoews thanks for the fixes and review!
I added a section about numpy array creation.

manager that can be copied into your project::

import contextlib
import shapely
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

If including this full code snippet might make the section a bit long to read, we could alternatively also link to the github issue's comment from where I copied this: #1096 (comment) (which might also make it easier to comment / ask questions about it on the issue)

@jorisvandenbossche
Copy link
Copy Markdown
Member Author

cc also @caspervdw @brendan-ward @martinfleis feedback on those docs is very welcome!

Copy link
Copy Markdown
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.

Beautiful writing @jorisvandenbossche. Is it okay if we go ahead with 1.8a3 before this is done and introduce this in 1.8b1?

@jorisvandenbossche
Copy link
Copy Markdown
Member Author

It's a doc only change, so it will be directly available at https://shapely.readthedocs.io/en/latest/ after merging (without a new release). So yes, you certainly don't have to wait on this one to cut a new alpha.

Copy link
Copy Markdown
Contributor

@martinfleis martinfleis left a comment

Choose a reason for hiding this comment

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

It looks really nice!

Copy link
Copy Markdown
Collaborator

@brendan-ward brendan-ward left a comment

Choose a reason for hiding this comment

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

This looks great, thanks for working on this @jorisvandenbossche !

A few minor comments to consider.


The ``array_interface()`` method and ``ctypes`` attribute will be removed in
Shapely 2.0, but since Shapely will start requiring NumPy as a dependency,
you can use NumPy or its array interface directly.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This might benefit from an example showing how to do this directly instead of line.array_interface().

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I am a bit hesitant to add an explicit example of accessing np.array(line.coords).__array_interface__, because I can't think of many good reasons you would need to do that directly yourself (also because it is somewhat legacy, with now the more general python buffer interface: https://numpy.org/doc/stable/reference/arrays.interface.html), but it would basically be:

In [1]: from shapely.geometry import LineString

In [2]: line = LineString([(0, 0), (1, 1), (2, 2)])

In [4]: coords = np.array(line.coords)

In [5]: coords.__array_interface__
Out[5]: 
{'data': (94533168670016, False),
 'strides': None,
 'descr': [('', '<f8')],
 'typestr': '<f8',
 'shape': (3, 2),
 'version': 3}

And numpy arrays also have a ctypes attribute, although it is a bit different:

In [6]: coords.ctypes
Out[6]: <numpy.core._internal._ctypes at 0x7f7ca9abdd00>

In [7]: coords.ctypes.data
Out[7]: 94533168670016

For this we could point to https://numpy.org/doc/stable/reference/generated/numpy.ndarray.ctypes.html

I could maybe at least point to those urls.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Pointing to those URLs is fine.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hope you don't mind using sphinx.ext.intersphinx for the numpy links. Outputs were checked locally to make sure they are the same.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Certainly not, thanks!

jorisvandenbossche and others added 5 commits August 24, 2021 08:12
Co-authored-by: Brendan Ward <bcward@astutespruce.com>
Co-authored-by: Brendan Ward <bcward@astutespruce.com>
Copy link
Copy Markdown
Collaborator

@caspervdw caspervdw left a comment

Choose a reason for hiding this comment

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

The migration guide looks solid @jorisvandenbossche , I have no comments or suggestions.

@jorisvandenbossche
Copy link
Copy Markdown
Member Author

OK, merging this. We can always further update the guide based on feedback / issues we notice.

@jorisvandenbossche jorisvandenbossche merged commit c391288 into shapely:master Sep 8, 2021
@jorisvandenbossche jorisvandenbossche deleted the migration-guide branch September 8, 2021 07:48
@jorisvandenbossche
Copy link
Copy Markdown
Member Author

Rendered version can now be seen here: https://shapely.readthedocs.io/en/latest/migration.html

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.

7 participants