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

Feature request: iterate over flattened features #692

Closed
dpmcmlxxvi opened this issue Apr 30, 2017 · 7 comments
Closed

Feature request: iterate over flattened features #692

dpmcmlxxvi opened this issue Apr 30, 2017 · 7 comments
Assignees
Milestone

Comments

@dpmcmlxxvi
Copy link
Collaborator

dpmcmlxxvi commented Apr 30, 2017

Turf has a flatten method that creates a feature collection. It would be helpful to have a flattenEach method that traverses them with a callback without allocating them and consuming memory. I can submit a PR if there is interest in this feature.

Note, this would likely introduce helpers as a dependency. The helpers module in turn has no dependencies. So, this shouldn't be a problem unless it is desired to keep all these low level modules free of any dependencies.

@DenisCarriere
Copy link
Member

👍 Agreed having callback functionality, however @turf/helpers shouldn't be used to extend other modules and should be kept with zero dependencies.

One option that won't break any backwards compatibility is:

  • Add optional callback param flatten(geojson, callback), if callback is present then method should return void. If callback is not present flatten(geojson) then method should return FeatureCollection/Feature.

@morganherlocker What's your opinion on including an optional callback param? Might be worth doing this to many other modules (I can already think of a few).

@dpmcmlxxvi
Copy link
Collaborator Author

Just to be clear, I meant adding @turf/helpers as a dependency to @turf/meta since meta is where all the other forEach like methods are located. I didn't mean adding dependencies to helpers. It seemed like that would be fine since @turf/helpers is used as a dependency for many other modules. But if it is desired to keep meta free of dependencies then adding it to flatten is another option.

@DenisCarriere
Copy link
Member

Adding the current implementation of @turf/flatten to @turf/meta would defeat the purpose of iterating over the features since the current state of flatten outputs a FeatureCollection. If this isn't changed in the @turf/flatten module then the memory issue would still exist.

I'm 👍 adding a callback param directly in @turf/flatten, however this would require a significant overhaul of the module.

@dpmcmlxxvi
Copy link
Collaborator Author

I didn't have in mind simply moving the flatten implementation to meta since as you mentioned it is geared toward creating new features at every step. Instead it would be a new implementation in meta that uses geomEach, which does most of the leg work, and then invoking the proper feature constructor, or iterating over sub-geometry coordinates if it is a multi-geometry and then invoking the feature constructor.

If I understand flatten correctly, the new implementation would make the flatten implementation much simpler. I think I have it worked out and can submit a PR for review.

@DenisCarriere
Copy link
Member

👍 Sounds like a plan! I completely agree that @turf/flatten can be simplified.

@dpmcmlxxvi Feel free to send a PR to the @turf/meta module with your proposed solution, looking forward to it!

@DenisCarriere
Copy link
Member

Implemented in Turf v4.3
PR: #712

@dpmcmlxxvi
Copy link
Collaborator Author

dpmcmlxxvi commented May 8, 2017

I still plan on updating @turf/flatten with new flattenEach. Just need to find some free cycles to get back to it soon.

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

No branches or pull requests

2 participants