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

flatten: Geometry Collection in Feature Collection does not flatten #709

Closed
2 tasks done
dpmcmlxxvi opened this issue May 3, 2017 · 6 comments
Closed
2 tasks done
Assignees
Labels
Milestone

Comments

@dpmcmlxxvi
Copy link
Collaborator

dpmcmlxxvi commented May 3, 2017

While working on a PR for #692 to create a flattenEach method I noticed that @turf/flatten does not appear to flatten a geometry collection that is inside a feature collection. See the jsfiddle.

Not sure if this is the intended behavior. The GeoJSON spec discourages nesting geometry collections but I see nothing about nesting a geometry collection in a feature collection. So, seems like a valid use case.

I was planning on having the PR for #692 iterate over the same features that are yielded using @turf/flatten so I'd like some clarification on what is the desired behavior. If the current implementation is a bug than I can use a proper flattening in the PR.

The fix for the current implementation seems simple enough. Have an additional check for a GeometryCollection type in the switch case of flattenFeatureCollection.

case 'MultiPoint':
case 'MultiLineString':
case 'MultiPolygon':
case 'GeometryCollection':
    featureEach(flatten(multiFeature), function (feature) {
        features.push(feature);
    });
break;
  • GeoJSON data as a gist file or geojson.io (filename extension must be .geojson).
  • Snippet of source code or for complex examples use jsfiddle.
@DenisCarriere
Copy link
Member

Never knew that GeometryCollection inside a FeatureCollection was a thing (seems like it is), you are correct by saying nested GeometryCollection shouldn't be allowed.

You might have issues using featureEach on a Geometry Collection since that will only return Features, you will need to use geomEach if you want to iterate over the Geometry Collection.

Question: Should the output remain a FeatureCollection regardless of the input geometry (open to suggestions)?

Input Output
Feature<Point> FeatureCollection<Point>
Feature<MultiPoint> FeatureCollection<Point>
FeatureCollection<MultiPoint> FeatureCollection<Point>
Geometry<Point> FeatureCollection<Point>
Geometry<MultiPoint> FeatureCollection<Point>
GeometryCollection<Point> FeatureCollection<Point>

@DenisCarriere
Copy link
Member

@dpmcmlxxvi I'm starting to get the same frustrations you're having using the @turf/flatten module. There's definitely a place for having flattenEach & flattenReduce in @turf/meta. 👍

@dpmcmlxxvi
Copy link
Collaborator Author

Agreed. I think having a FeatureCollection<Point|LineString|Polygon> as output regardless of input would be the most expected result. At least to me that's what the word flatten most suggests - reducing everything to a single geometries.

@DenisCarriere
Copy link
Member

DenisCarriere commented May 4, 2017

👍 Another thing I noticed with flatten is when you provide a Geometry Object it returns a FeatureCollection with Geometries (not Features).

@dpmcmlxxvi Let me know if you want to tackle adding flattenEach & flattenReduce to @turf/meta or do you want me to take care of it? Most of the logic is already done, just need to piece it all together.

Once this is finished, @turf/flatten will simply be using flattenEach and return a FeatureCollection<any> (will probably end up being 5 lines of code) 👍 .

@dpmcmlxxvi
Copy link
Collaborator Author

@DenisCarriere Yeah, I'll be submitting the PR pretty soon. Agree on it hopefully simplifying the flatten code. All the crucial logic is already being done in geomEach.

@dpmcmlxxvi
Copy link
Collaborator Author

Flattening GeometryCollection in FeatureCollection is now supported in @turf/meta/flattenEach @turf/flatten with updates #712 and #724. In fact, it should now support any valid GeoJSON.

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

2 participants