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

redo turf-isolines #390

Closed
morganherlocker opened this issue Jun 1, 2016 · 5 comments
Closed

redo turf-isolines #390

morganherlocker opened this issue Jun 1, 2016 · 5 comments
Assignees

Comments

@morganherlocker
Copy link
Member

The existing implementation is finicky and now relies on outdated internal dependencies.

@rowanwins
Copy link
Member

Just noting here that @stefanogithub has a pull request open on this in the old turf-isobands repo.

@stebogit
Copy link
Collaborator

@DenisCarriere do I understand correctly that the isolines module has currently issues/bug as well? Don't get exactly which ones though.
Would you like me to take a look into this and maybe trying to replace conrec with marchingsquare.isolines?

@stebogit
Copy link
Collaborator

@DenisCarriere Looking at the module's test it seems to me that the output is incorrect... am I wrong?

Other consideration: what's actually the difference between an isoline and an isoband?
The more I think about this it seems to me the isoline is just a particular case of isoband, where its two limits (upper and lower) coincide.
Couldn't we just use the same algorithm for both isobands and isolines? It would be probably mainly a matter of output format, MultiPolygons vs. LinearRing or LineString.
In this example for instance it's also clear that the conrec algorithm/library does not "group" isolines of the same level, considering each line indipendent. I don't think this is a desired result for whom needs isolines.

@DenisCarriere
Copy link
Member

Well ideally we should only have 1 module, the only difference is the Polygon & LineString outputs.

Once we get isobands up and running we could drop isolines (turf 4.x) in favor of isobands.

@morganherlocker
Copy link
Member Author

Well ideally we should only have 1 module, the only difference is the Polygon & LineString outputs.

Note that we used to have isobands and isolines, but isobands was deprecated due to limitations in the conrec implementation that did not apply to LineString output. Having both might be useful, and turning one format into the other can be surprisingly tricky for people looking for a quick result.

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

4 participants