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

Crash in WKT of MultiPoint including an empty Point #305

Closed
jorisvandenbossche opened this issue Apr 17, 2020 · 5 comments
Closed

Crash in WKT of MultiPoint including an empty Point #305

jorisvandenbossche opened this issue Apr 17, 2020 · 5 comments

Comments

@jorisvandenbossche
Copy link
Contributor

Using the Python pygeos package to show the error (from pygeos/pygeos#134):

# creating a MultiPoint with that includes an empty Point
import pygeos

point = pygeos.points(1,1)  
box = pygeos.box(2,2,4,4) 
point_empty = pygeos.intersection(p, box)

mp_empty  = pygeos.multipoints([point, point_empty])                  

Converting to WKB raises an informative error:

>>> pygeos.to_wkb(mp_empty)      
...
GEOSException: IllegalArgumentException: Empty Points cannot be represented in WKB

However, converting to WKT crashes:

>>> pygeos.to_wkt(mp_empty)    
Segmentation fault (core dumped)

I am not 100% sure this is a GEOS issue, since there might be something wrong on the PyGEOS side. But posting here in case somebody knows or can confirm this is a GEOS bug.

@pramsey
Copy link
Member

pramsey commented Apr 20, 2020

@pramsey
Copy link
Member

pramsey commented Apr 20, 2020

Patched in 3.7, 3.8, master.

a73568c

@pramsey pramsey closed this as completed Apr 20, 2020
@jorisvandenbossche
Copy link
Contributor Author

Thanks for the quick fix!

One thing I am wondering, is the reading of such WKT output supported? (or should it be supported?)

In [5]: pygeos.from_wkt("MULTIPOINT (EMPTY, 1 2)")  
---------------------------------------------------------------------------
GEOSException                             Traceback (most recent call last)
<ipython-input-5-8a845f13df47> in <module>
----> 1 pygeos.from_wkt("MULTIPOINT(EMPTY, 1 1)")

~/scipy/repos/pygeos/pygeos/io.py in from_wkt(geometry, **kwargs)
    158     <pygeos.Geometry POINT (0 0)>
    159     """
--> 160     return lib.from_wkt(geometry, **kwargs)
    161 
    162 

GEOSException: ParseException: Unexpected token: WORD EMPTY

(using GEOS 3.8.0)

@pramsey
Copy link
Member

pramsey commented Apr 20, 2020

Current reading it is not supported. JTS does not read it either. PostGIS does. Hard call. It would have been way easier to write the test if reading was supported. :)

@jorisvandenbossche
Copy link
Contributor Author

jorisvandenbossche commented Apr 20, 2020

I would vote (if it mattered ;)) for consistency within GEOS: either support it for both reading and writing, or not support it. It could also raise with a nice error message (like writing to WKB does), instead of crashing as before.

Personally, I think it would be nice to have some of the extensions that are in PostGIS in GEOS (also eg for WKB of empty points), instead of having PostGIS, sf, Shapely, etc all implement their own similar workaround.
But I can also certainly understand if GEOS rather wants to stick strictly to the standard.

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

No branches or pull requests

2 participants