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

@turf/concave improvements #910

Closed
2 tasks done
stebogit opened this issue Aug 20, 2017 · 5 comments
Closed
2 tasks done

@turf/concave improvements #910

stebogit opened this issue Aug 20, 2017 · 5 comments

Comments

@stebogit
Copy link
Collaborator

stebogit commented Aug 20, 2017

  • set maxEdge parameter optional and by default to Infinity.
    A common expected output of a concave function (like concaveman) is always to return a single concave hull Polygon. @turf/concave has the capability to return a multipolygon (with holes! 👍 ), but this should not be the default behaviour.

  • remove Error output if there is no hull to return, it should return an empty Feature (helpers.feature(null)). (done here).
    Currently the Error "too few polygons found to compute concave hull" is likely due to a small maxEdge. This shouldn't be considered an error, because an error should mean a misuse of the module (which isn't the case here).

Ref. #907.

@DenisCarriere
Copy link
Member

👍

@stebogit
Copy link
Collaborator Author

@DenisCarriere are we sure we should return a helpers.feature(null) if no concave hull is possible?
I'd say it should return null, for the same reasons why we have now null from @turf/difference and @turf/intersect

@stebogit stebogit assigned stebogit and DenisCarriere and unassigned stebogit Sep 27, 2017
@DenisCarriere
Copy link
Member

@stebogit 👍 Agreed, I forgot about this module having feature(null).

If there's any other modules returning a single Feature with Null geometry, it should be changed.

@DenisCarriere
Copy link
Member

DenisCarriere commented Sep 27, 2017

Also seems like the @turf/great-circle is doing the same:

Arc.prototype.json = function () {
if (this.geometries.length <= 0) {
return {'geometry': { 'type': 'LineString', 'coordinates': null },
'type': 'Feature', 'properties': this.properties
};
} else if (this.geometries.length === 1) {
return {'geometry': { 'type': 'LineString', 'coordinates': this.geometries[0].coords },
'type': 'Feature', 'properties': this.properties
};

@stebogit
Copy link
Collaborator Author

Implemented with #1020 and acbe77d

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