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

discrepancy between code and documentation for union #1669

Closed
andrewharvey opened this issue May 2, 2019 · 5 comments
Closed

discrepancy between code and documentation for union #1669

andrewharvey opened this issue May 2, 2019 · 5 comments

Comments

@andrewharvey
Copy link
Contributor

The JSDoc for turn.union at https://github.com/Turfjs/turf/blob/master/packages/turf-union/index.ts says:

  • Takes two {@link (Multi)Polygon(s)} and returns a combined polygon. If the input polygons are not contiguous, this function returns a {@link MultiPolygon} feature.

However the online documentation at http://turfjs.org/docs/#union says:

Takes two or more polygons

which is wrong, as union only takes two polygons, as two arguments, not any number of polygons as the live documentation suggests.

I'm not sure where I need to place the PR to fix this, given it's already fixed in master.

@lostpebble
Copy link

I've been making use of union recently and passing in multiple polygons like so:

const newGeometryFeature = union(...geometryFeatures);

It seems to be combining more than two polygon features together. But looking at the code in master, it seems like it only takes two? Kinda weird... Does the master code and released version not align?

I'm using "@turf/turf": "5.1.6".

@andrewharvey
Copy link
Contributor Author

andrewharvey commented May 3, 2019

const newGeometryFeature = union(...geometryFeatures);

That's what I tried the first time because of the documentation, but it failed, confused why I diged into the source code to find that it only takes two feature arguments. Instead I had use union inside an Array.reduce function to get the same affect.

It seems to be combining more than two polygon features together. But looking at the code in master, it seems like it only takes two? Kinda weird... Does the master code and released version not align?

Yeah that's what I'm very confused about too!

@rowanwins
Copy link
Member

Hi @andrewharvey & @lostpebble

Sorry for the confusion. Some of those docs are probably reflectively of the change from JSTS which was used in v5 and earlier which took any number of arguments. In v6 martinez was used which only takes 2 arguments.

In v7 I've implemented polygon-clipping which goes back to taking any number of arguments. The latest hasn't been published yet as a turf module yet but the 0.14.0 release of polygon-clipping looks really good and passes all the turf tests.

@brylie
Copy link

brylie commented Aug 18, 2020

This almost seems like a regression. Is there a way to get a union of a MultiPolygon with Turf.js?

@rowanwins
Copy link
Member

Docs and code are now in sync. It's still on my radar to relook at adding support for more than 2 polys.

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

4 participants