Initial commit of turf/boolean-within #924

rowanwins · 2017-08-29T13:04:26Z

New module `@turf/boolean-within`

/**
 * Boolean-within returns true if the first geometry is completely within the second geometry.
 * The interiors of both geometries must intersect and, the interior and boundary of the primary (geometry a)
 * must not intersect the exterior of the secondary (geometry b).
 * Boolean-within returns the exact opposite result of the `@turf/boolean-contains`.
 *
 * @name booleanWithin
 * @param {Geometry|Feature<any>} feature1 GeoJSON Feature or Geometry
 * @param {Geometry|Feature<any>} feature2 GeoJSON Feature or Geometry
 * @returns {boolean} true/false
 * @example
 * const line = turf.lineString([[1, 1], [1, 2], [1, 3], [1, 4]]);
 * const point = turf.point([1, 2]);
 *
 * turf.booleanWithin(point, line);
 * //=true
 */
module.exports = function (feature1, feature2) {

Use a meaningful title for the pull request. Include the name of the package modified.
Have read How To Contribute.
Run npm test at the sub modules where changes have occurred.
Run npm run lint to ensure code style at the turf module level.

Submitting a new TurfJS Module.

Overview description of proposed module.
Include JSDocs with a basic example.
Execute ./scripts/generate-readmes to create README.md.
Add yourself to contributors in package.json using "Full Name <@github Username>".

Initial commit of the boolean-within module.

Tested against jsts using boolean-jsts with no issues. Tried testing using boolean-shapely but for some reason none of the tests resulting in true would pass.... probably something silly I did, all the fail tests worked fine...

Let me know if you spot anything astray.

/**
 * Benchmark Results
 *
 * LineIsNotWithinLine x 2,878,664 ops/sec ±7.20% (68 runs sampled)
 * LineIsNotWIthinPolygon x 1,818,294 ops/sec ±6.20% (64 runs sampled)
 * LineIsNotWIthinPolygonBoundary x 297,332 ops/sec ±6.31% (77 runs sampled)
 * MultiPointsIsNotWIthinLine x 2,767,186 ops/sec ±6.88% (73 runs sampled)
 * MultiPointsOnLineEndsIsNotWIthinLine x 1,606,670 ops/sec ±7.38% (62 runs sampled)
 * MultiPointIsNotWithinMultiPoint x 7,711,700 ops/sec ±10.08% (52 runs sampled)
 * MultiPointAllOnBoundaryIsNotWithinPolygon x 435,926 ops/sec ±6.12% (49 runs sampled)
 * MultiPointIsNotWithinPolygon x 1,709,920 ops/sec ±7.40% (57 runs sampled)
 * PointIsNotWithinLine x 4,148,704 ops/sec ±5.95% (53 runs sampled)
 * PointIsNotWithinLineBecauseOnEnd x 5,243,476 ops/sec ±7.68% (68 runs sampled)
 * PointOnEndIsWithinLinestring x 5,178,472 ops/sec ±7.04% (68 runs sampled)
 * PointIsNotWithinMultiPoint x 12,366,325 ops/sec ±6.98% (58 runs sampled)
 * PointIsNotWithinPolygon x 1,490,757 ops/sec ±11.13% (66 runs sampled)
 * PointOnPolygonBoundary x 1,758,292 ops/sec ±6.62% (63 runs sampled)
 * Polygon-Polygon x 1,406,871 ops/sec ±8.49% (60 runs sampled)
 * LineIsWithinLine x 2,436,801 ops/sec ±7.33% (59 runs sampled)
 * LinesExactSame x 1,815,051 ops/sec ±10.68% (60 runs sampled)
 * LineIsContainedByPolygon x 330,016 ops/sec ±6.95% (54 runs sampled)
 * LineIsContainedByPolygonWithNoInternalVertices x 465,371 ops/sec ±6.99% (61 runs sampled)
 * MultipointsIsWithinLine x 1,742,280 ops/sec ±7.25% (62 runs sampled)
 * MultiPointsWithinMultiPoints x 11,546,059 ops/sec ±4.41% (87 runs sampled)
 * MultiPointIsWithinPolygon x 409,210 ops/sec ±8.21% (53 runs sampled)
 * PointIsWithinLine x 7,298,078 ops/sec ±10.89% (73 runs sampled)
 * PointIsWithinMultiPoint x 16,495,443 ops/sec ±7.21% (68 runs sampled)
 * PointIsWithinPolygon x 1,820,036 ops/sec ±9.11% (60 runs sampled)
 * PolygonIsWIthinPolygon x 518,524 ops/sec ±5.57% (80 runs sampled)
 * PolygonsExactSameShape x 418,247 ops/sec ±7.43% (80 runs sampled)

DenisCarriere · 2017-08-29T15:50:35Z

Tried testing using boolean-shapely but for some reason none of the tests resulting in true would pass.... probably something silly I did, all the fail tests worked fine...

I'll try to make the boolean-shapely in sync instead of returning a Promise, which will make it easier for the tape testing.

DenisCarriere · 2017-08-29T15:51:24Z

packages/turf-boolean-within/index.js

+ * must not intersect the exterior of the secondary (geometry b).
+ * Boolean-within returns the exact opposite result of the `@turf/boolean-contains`.
+ *
+ * @name booleanWith


booleanWithin

gees your diligent!

Haha! lolz..

DenisCarriere · 2017-08-29T15:52:39Z

packages/turf-boolean-within/index.js

+        case 'Polygon':
+            return inside(geom1, geom2, true);
+        default:
+            throw new Error('feature2 ' + type2 + ' geometry not supported');


~~Instead of throwing an error, it would be better to throw false if Point cannot be within another Point.~~

Hmmm this is the pattern we've followed for all the other boolean modules. I'd suggest if we want to change our approach for one then we should do it for all of them. Perhaps we should do this as a v5 change?

Actually, no might be fine... since some of the within combinations don't actually make sense, this is good for now. 👍

DenisCarriere · 2017-08-29T15:52:55Z

packages/turf-boolean-within/index.js

+        case 'Polygon':
+            return isMultiPointInPoly(geom1, geom2);
+        default:
+            throw new Error('feature2 ' + type2 + ' geometry not supported');


Same as above

DenisCarriere · 2017-08-29T15:53:43Z

packages/turf-boolean-within/index.js

+            throw new Error('feature2 ' + type2 + ' geometry not supported');
+        }
+    default:
+        throw new Error('feature1 ' + type1 + ' geometry not supported');


No support for MultiPolygon or MultiLineString?

Not yet, but there probably should be. The problem is there aren't any picture for it and so I always forget :)

HAHA no pictures 🤣 Those pictures are so helpful!

DenisCarriere · 2017-08-29T15:55:27Z

packages/turf-boolean-within/test.js

+        const result = within(feature1, feature2);
+        if (process.env.JSTS) t.true(booleanJSTS('within', feature1, feature2), '[true] JSTS - ' + name);
+
+        if (process.env.SHAPELY) shapely.within(feature1, feature2).then(result => t.true(result, '[true] shapely - ' + name));


I'm going to try to make this look like this:

t.true(shapely.within(feature1, feature2), '[true] shapely - ' + name);

That would be nicer :)

Agreed! I got to get on that...

DenisCarriere

Overall looks great! Would be nice to know why those shapely boolean tests didn't pass, i'll look into that.

…boolean-within

DenisCarriere · 2017-09-01T17:45:31Z

Seems like something crapped out on Mourner's eslint config: 🤔

mourner:
	Configuration for rule "indent" is invalid:
	Value "data["1"].flatTernaryExpressions" has additional properties.
Referenced from: /home/travis/build/Turfjs/turf/.eslintrc.js
Error: mourner:
	Configuration for rule "indent" is invalid:
	Value "data["1"].flatTernaryExpressions" has additional properties.

stebogit · 2017-09-01T20:01:02Z

@DenisCarriere that's what happened here!! 😢
So it's not my fault 😥 😄

DenisCarriere · 2017-09-01T20:28:42Z

@stebogit No it's not your fault 😄 Strange things happen with NPM modules, that's why locking them into the version is sometimes the best thing to do... I've locked in TurfJS's estlint dependencies to v2.0.0. Didn't look into what would of caused that.

DenisCarriere · 2017-09-05T03:29:47Z

packages/turf-simplify/test.js

@@ -4,7 +4,7 @@ const path = require('path');
 const load = require('load-json-file');
 const write = require('write-json-file');
 const truncate = require('@turf/truncate');
-const {polygon, multiPolygon} = require('@turf/helpers');


@rowanwins Don't know why @turf/simplify was modified in this PR. 🤷‍♂️

rowanwins and others added 4 commits August 29, 2017 22:59

Initial commit of turf/boolean-within

d6575de

Add readme

bb4506a

Add yarn lock file

8f2b441

Merge branch 'master' into boolean-within

f74f03b

DenisCarriere reviewed Aug 29, 2017

View reviewed changes

DenisCarriere approved these changes Aug 29, 2017

View reviewed changes

rowanwins added 2 commits August 30, 2017 19:20

Fix jsdoc name

2a5e73a

Merge branch 'boolean-within' of /~https://github.com/Turfjs/turf into …

1870711

…boolean-within

DenisCarriere assigned rowanwins Sep 1, 2017

DenisCarriere added the new-module label Sep 1, 2017

DenisCarriere added this to the 4.7.0 milestone Sep 1, 2017

DenisCarriere added 2 commits September 1, 2017 13:45

Merge branch 'master' into boolean-within

fb686a8

Merge branch 'master' into boolean-within

87da74c

DenisCarriere added 3 commits September 4, 2017 17:05

Add types tests

1b94521

Add booleanWithin to @turf/turf

8e9af20

Merge branch 'master' into boolean-within

24dee26

DenisCarriere reviewed Sep 5, 2017

View reviewed changes

Fix simplify test

a23ed48

DenisCarriere merged commit e3785ea into master Sep 5, 2017

DenisCarriere deleted the boolean-within branch September 5, 2017 03:44

DenisCarriere mentioned this pull request Sep 5, 2017

New minor release! Turf 4.7.1 🎉 #932

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial commit of turf/boolean-within #924

Initial commit of turf/boolean-within #924

rowanwins commented Aug 29, 2017 •

edited by DenisCarriere

Loading

DenisCarriere commented Aug 29, 2017

DenisCarriere Aug 29, 2017

rowanwins Aug 30, 2017

DenisCarriere Sep 1, 2017

DenisCarriere Aug 29, 2017 •

edited

Loading

rowanwins Aug 30, 2017

DenisCarriere Sep 1, 2017

DenisCarriere Aug 29, 2017

DenisCarriere Aug 29, 2017

rowanwins Aug 30, 2017

DenisCarriere Sep 1, 2017

DenisCarriere Aug 29, 2017

rowanwins Aug 30, 2017

DenisCarriere Sep 1, 2017

DenisCarriere left a comment

DenisCarriere commented Sep 1, 2017

stebogit commented Sep 1, 2017

DenisCarriere commented Sep 1, 2017

DenisCarriere Sep 5, 2017

Initial commit of turf/boolean-within #924

Initial commit of turf/boolean-within #924

Conversation

rowanwins commented Aug 29, 2017 • edited by DenisCarriere Loading

New module @turf/boolean-within

DenisCarriere commented Aug 29, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DenisCarriere Aug 29, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DenisCarriere left a comment

Choose a reason for hiding this comment

DenisCarriere commented Sep 1, 2017

stebogit commented Sep 1, 2017

DenisCarriere commented Sep 1, 2017

Choose a reason for hiding this comment

rowanwins commented Aug 29, 2017 •

edited by DenisCarriere

Loading

New module `@turf/boolean-within`

DenisCarriere Aug 29, 2017 •

edited

Loading