terraformer-js / terraformer

A geographic toolkit for dealing with geometry, geography, formats, and building geodatabases
MIT License
182 stars 28 forks source link

Error with "intersects" method when using Feature Collections. #83

Closed rgwozdz closed 2 years ago

rgwozdz commented 2 years ago

When attempting an intersects with multipolygon and polygon arguments, I get a fatal error:

const { intersects }= require("@terraformer/spatial")

const multipolygon = {"type":"FeatureCollection","features":[{"type":"Feature","properties":{},"geometry":{"type":"MultiPolygon","coordinates":[[[[-101.0,37.0],[-99.0,37.0],[-99.0,38.0],[-101.0,38.0],[-101.0,37.0]]],[[[-97.0,35.0],[-95.0,35.0],[-95.0,36.0],[-97.0,36.0],[-97.0,35.0]]]]}},{"type":"Feature","properties":{},"geometry":{"type":"Polygon","coordinates":[[[-104,33],[-100,33],[-100,34],[-104,34],[-104,33]]]}}]}

const geometryFilter = {"type":"FeatureCollection","features":[{"type":"Feature","properties":{},"geometry":{"type":"Polygon","coordinates":[[[-98.5,34.6],[-94,34.6],[-94,37],[-98.5,37],[-98.5,34.6]]]}}]}

intersects(multipolygon, geometryFilter)

Results:

Screen Shot 2022-07-05 at 1 14 23 PM

jgravois commented 2 years ago

hey @rgwozdz!

currently the library pulls out the corresponding geometry when one passes in a single GeoJSON Feature, but doesn't appear to account for input FeatureCollections in any meaningful way.

in contrast, 'real' MultiPolygon's are definitely supported as input.

my first instinct is that it'd be a bit presumptious for terraformer to assume a FeatureCollection was composed of multiple features that mimic the relationship of individual polygons within a multipolygon, but i'm happy to be convinced otherwise.

regardless, there's definitely more that could be done in the way of clarifying expected behavior so i'd be happy to steward a contribution in that form too.

rgwozdz commented 2 years ago

@jgravois - ok, this isn't a big deal to me; I actually happened upon the error as I was digging into some other behavior with intersects. I'm happy to leave as is, but maybe we should update the doc as it appears to state that FeatureCollection is an allowable input. If you think that update is a good idea, let me know, and I can PR.

jgravois commented 2 years ago

this isn't a big deal to me

understood. i definitely do appreciate you making notes and leaving breadcrumbs as you work.

PRs with tweaks to the doc in the name of clarification are always welcome. if you or a colleague end up poking around in intersects() code someday and want to codify the expectation with regard to FeatureCollections too, all the better. 🙏

fwiw, something simple to document the behavior in https://github.com/terraformer-js/terraformer/issues/71 is warranted too.

rgwozdz commented 2 years ago

Cool will do; updated the name of the issue here. I'm actually seeing something possibly(?) unexpected with intersects when I pass in simple multipolygon geometries. I'll put it in a a fresh issue though.

jgravois commented 2 years ago

my plan is to publish a release as soon as this small doc issue and #68 are addressed.

jgravois commented 2 years ago

resolved by #89 🌹