-
Notifications
You must be signed in to change notification settings - Fork 399
DistanceOp::distance(): make it return +inf instead of 0 if one of the geometry is empty #1345
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
base: main
Are you sure you want to change the base?
Conversation
… geometry is empty Fixes OSGeo/gdal#12978
pramsey
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LgTM
|
Note GEOS current behavior regarding empty geometries and distance reflects JTS one (CC @dr-jts ) |
| } | ||
| if(geom[0]->isEmpty() || geom[1]->isEmpty()) { | ||
| return 0.0; | ||
| return std::numeric_limits<double>::quiet_NaN(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is (/was?) a PostGIS page that explains this difference between the GEOS and SFCGAL backends. GEOS returned 0 (before this PR), while SFCGAL returned Inf.
When I asked why Inf rather than NaN, I was told that:
- NaN: was for invalid operations or errors.
- Inf: the geometry exists, but it does not contain any points.
So, here, I also tend to lean towards Inf, as you suggested in your comment Calling Distance on an empty geometry returns 0 OSGeo/gdal#12978 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with Inf too
… geometry is empty
|
Changed to returning +Inf |
Fixes OSGeo/gdal#12978