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

segmentation fault using float("nan") in parallel_offset #1483

Closed
crschmidt opened this issue Aug 16, 2022 · 5 comments
Closed

segmentation fault using float("nan") in parallel_offset #1483

crschmidt opened this issue Aug 16, 2022 · 5 comments
Labels
Milestone

Comments

@crschmidt
Copy link

Expected behavior and actual behavior.

Using a nan as a parallel_offset causes a segmentation fault.

Expected the function to return e.g. a ctypes.ArgumentError: argument 3: <class 'TypeError'>: wrong type (as happens if you pass a string "nan")

Steps to reproduce the problem.

from shapely.geometry import shape
shape({"type": "LineString", "coordinates": [[0,0],[0,1]]}).parallel_offset(float("nan"))

Operating system

Linux

Shapely version and provenance

1.8.2 installed form pypi using pip

@mwtoews
Copy link
Member

mwtoews commented Aug 16, 2022

There are potentially several other similar scenarios, e.g. .buffer(float("nan")) or .buffer(float("inf")), etc. so potentially inputs could be checked for several other functions.

Also, shapely 2.0 does not crash, but will either return None for float("nan"), or raise "shapely.errors.GEOSException: IllegalArgumentException: CGAlgorithmsDD::orientationIndex encountered NaN/Inf numbers" for float("inf") which slightly better (but curiously not shown for "nan"). I would rater see GEOS catch and handle these inputs, raising a more-direct exception message.

@crschmidt
Copy link
Author

Yeah, initially I thought that ctypes might have just been choking on creating a type correctly, but it seems that ctypes is fine with handling nans coming at it, so this probably is better fixed at the GEOS level. But I did see someone say in an issue that any segmentation fault in shapely should be considered a bug, so wanted to pass it your way :)

@mwtoews mwtoews added the bug label Aug 16, 2022
@mwtoews
Copy link
Member

mwtoews commented Aug 16, 2022

Yes, seg faults should always be reported, crashes are bad, exceptions should be raised, etc., so thanks for the issue.

As for shapely-1.8 with ctypes, I'm not sure how much appetite there is for checking inputs, as this release series is about to be replaced by shapely-2.0. And for 2.0, it would be nice to see some consistency.

See libgeos/geos#661 to improve upstream exception messages which would benefit shapely-2.0

@jorisvandenbossche
Copy link
Member

Small note regarding the 2.0 behaviour: the reason that NaN behaves differently in this case is because NaN input is handled on our side in the C code, and in that case we see this as a "missing" value, and never call the GEOS function but directly return None:

shapely/src/ufuncs.c

Lines 1430 to 1434 in 9582f30

if ((in1 == NULL) || npy_isnan(width)) {
// in case of a missing value: return NULL (None)
geom_arr[i] = NULL;
} else {
geom_arr[i] = GEOSOffsetCurve_r(ctx, in1, width, quadsegs, joinStyle, mitreLimit);

While inf input is passed through to GEOS.

@sgillies
Copy link
Contributor

Resolved in 1.8.5.

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

No branches or pull requests

4 participants