Skip to content

Clean-up use of impl/lgeos/delegated/exceptNull in shapely/geometry/#1020

Merged
jorisvandenbossche merged 8 commits intoshapely:shapely-2.0from
jorisvandenbossche:refactor-cleanup-base-delegated
Nov 27, 2020
Merged

Clean-up use of impl/lgeos/delegated/exceptNull in shapely/geometry/#1020
jorisvandenbossche merged 8 commits intoshapely:shapely-2.0from
jorisvandenbossche:refactor-cleanup-base-delegated

Conversation

@jorisvandenbossche
Copy link
Copy Markdown
Member

@jorisvandenbossche jorisvandenbossche commented Nov 5, 2020

Follow-up on #983, cleaning up a few more things in geometry/base.py

  • The delegated decorator is no longer needed (no support for another "impl"), also removed the tests for this
  • project is already implemented by PyGEOS, so using that one (same for relate and relate_pattern now)
  • Remove the exceptNull decorator, as such error handling is now handled in pygeos C code. For interpolate, we only have a slight difference in behaviour, though (for which I updated the test), see inline comment.
  • Removed self.impl, since this is now no longer rused


assert empty_line.is_empty
interpolated_point = empty_line.interpolate(.5, normalized=True)
assert interpolated_point.is_empty
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.

This test/behaviour comes from #653. GEOS segfaults when passing an empty linestring, and Shapely solved this with the exceptNull decorator checking if any of the arguments was empty. PyGEOS currently also checks for this explicitly (to avoid the segfault, see https://github.com/pygeos/pygeos/blob/32397d998d2fc7fe3afef673539dace9fd8efe7f/src/geos.c#L297), but we decided to return an empty point instead of raising an error (see https://github.com/pygeos/pygeos/blob/32397d998d2fc7fe3afef673539dace9fd8efe7f/src/ufuncs.c#L373)

Any strong opinions on the desired behaviour?

cc @snorfalorpagus

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.

This change of behavior is welcome, particularly since this is what PostGIS does:

db=> SELECT ST_AsText(ST_LineInterpolatePoint('LINESTRING EMPTY'::geometry, 0.2));
  st_astext
-------------
 POINT EMPTY
(1 row)

@coveralls
Copy link
Copy Markdown

coveralls commented Nov 5, 2020

Coverage Status

Coverage decreased (-1.08%) to 76.597% when pulling ccd5995 on jorisvandenbossche:refactor-cleanup-base-delegated into e0d3c53 on Toblerity:shapely-2.0.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.5%) to 74.847% when pulling 69d2fe2 on jorisvandenbossche:refactor-cleanup-base-delegated into 4f28db9 on Toblerity:shapely-2.0.

@jorisvandenbossche jorisvandenbossche force-pushed the refactor-cleanup-base-delegated branch from 69d2fe2 to bfe4149 Compare November 9, 2020 08:06
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.

Looks good. I see the remaining delegated functions are in shapely.prepared, which I suspect will be addressed later (as discussed in shapely/shapely-rfc#1)

@jorisvandenbossche
Copy link
Copy Markdown
Member Author

I see the remaining delegated functions are in shapely.prepared

Indeed, and the same for the impl / DefaultImplementation, which is now (with the latest updates to this PR) also only used in the shapely.prepared module. So fully cleaned usage of it in geometry/base.py, but we will only be able to fully remove all the code in impl.py if also the prepared module is updated to use pygeos (now, functionality related to "prepared" has been merged in pygeos, so that should be possible now).

@jorisvandenbossche jorisvandenbossche changed the title Clean-up use of delegated/impl/exceptNull in geometry/base.py Clean-up use of impl/lgeos/delegated/exceptNull in geometry/base.py Nov 24, 2020
@jorisvandenbossche jorisvandenbossche changed the title Clean-up use of impl/lgeos/delegated/exceptNull in geometry/base.py Clean-up use of impl/lgeos/delegated/exceptNull in shapely/geometry/ Nov 24, 2020
@jorisvandenbossche
Copy link
Copy Markdown
Member Author

Also removed any remaining use of lgeos in shapely/geometry/ submodule, except for the usage in pickling of LinearRings

@jorisvandenbossche
Copy link
Copy Markdown
Member Author

Needed to do a force-push to this branch, because I rebased the shapely-2.0 branch on latest master

@jorisvandenbossche
Copy link
Copy Markdown
Member Author

CI is failing because of a build failure with pygeos (opened pygeos/pygeos#260 for that).

@jorisvandenbossche jorisvandenbossche force-pushed the refactor-cleanup-base-delegated branch from 0d9afea to ccd5995 Compare November 27, 2020 10:00
@jorisvandenbossche
Copy link
Copy Markdown
Member Author

Actually, GEOSVERSION changed to GEOS_VERSION, so that was the cause ;)

@jorisvandenbossche
Copy link
Copy Markdown
Member Author

OK, remaining failure is a doctest failure unrelated to this PR, so will fix that in a separate PR (we started checking the doctests on master, which was now brought into shapely-2.0 branch, and there are some failures now)

jorisvandenbossche added a commit to jorisvandenbossche/shapely that referenced this pull request Apr 5, 2021
jorisvandenbossche added a commit to jorisvandenbossche/shapely that referenced this pull request May 27, 2021
jorisvandenbossche added a commit to jorisvandenbossche/shapely that referenced this pull request May 27, 2021
jorisvandenbossche added a commit to jorisvandenbossche/shapely that referenced this pull request Aug 20, 2021
jorisvandenbossche added a commit to jorisvandenbossche/shapely that referenced this pull request Sep 30, 2021
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.

3 participants