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

What to make of heterogenous Turf.combine test? #1756

Closed
langsmith opened this issue Sep 9, 2019 · 2 comments
Closed

What to make of heterogenous Turf.combine test? #1756

langsmith opened this issue Sep 9, 2019 · 2 comments

Comments

@langsmith
Copy link

I'm wondering what to make of the heterogenous test at https://github.com/Turfjs/turf/blob/master/packages/turf-combine/test.js#L154-L203.

I'm finishing up a Java port of the method and unsure how to interpret what this test means. Based on the docs at http://turfjs.org/docs/#combine and the other tests in https://github.com/Turfjs/turf/blob/master/packages/turf-combine/test.js, it appears as if a Point mixed with a Polygon and MultiPolygon, would not be allowed by this method.

The Point is added at https://github.com/Turfjs/turf/blob/master/packages/turf-combine/test.js#L188, but the Point's coordinates aren't checked at https://github.com/Turfjs/turf/blob/master/packages/turf-combine/test.js#L195-L200

Should the Point be ignored if the other geometries are homogeneous? E.g. would a LineString be ignored if a FeatureCollection also had two MultiPolygons?

@dpmcmlxxvi
Copy link
Collaborator

@langsmith It appears from the code the that intent is to allow heterogeneous collections. In fact, the code explicitly creates a group for each basic type (multipoint, multiline, and multipolygon) and includes output data for each type for which there was data of that type in the input collection.

As for why the coordinates of the point were not included in the test, my guess is that it was just an oversight by whomever wrote the test. But the test does explicitly check the multipoint type is included in the output, so it should be there.

@langsmith
Copy link
Author

Got it. Ok so, the final FeatureCollection in that test should theoretically have a MultiPoint (containing that single Point) and a MultiPolygon (containing the Polygon and MultiPolygon).

Thanks!

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