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

Stop turf-mask mutating by default, make it an option #2635

Merged
merged 13 commits into from
Aug 8, 2024
Merged
9 changes: 8 additions & 1 deletion packages/turf-mask/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ ring polygon with holes.

* `polygon` **([Polygon][1] | [MultiPolygon][2] | [Feature][3]<([Polygon][1] | [MultiPolygon][2])> | [FeatureCollection][4]<([Polygon][1] | [MultiPolygon][2])>)** GeoJSON polygon used as interior rings or holes
* `mask` **([Polygon][1] | [Feature][3]<[Polygon][1]>)?** GeoJSON polygon used as the exterior ring (if undefined, the world extent is used)
* `options` **[Object][5]** Optional parameters (optional, default `{}`)

* `options.mutate` **[boolean][6]** allows the `mask` GeoJSON input to be mutated (performance improvement if true) (optional, default `false`)

### Examples

Expand All @@ -24,7 +27,7 @@ const masked = turf.mask(polygon, mask);
const addToMap = [masked]
```

Returns **[Feature][3]<[Polygon][1]>** Masked Polygon (exterior ring with holes).
Returns **[Feature][3]<[Polygon][1]>** Masked Polygon (exterior ring with holes)

[1]: https://tools.ietf.org/html/rfc7946#section-3.1.6

Expand All @@ -34,6 +37,10 @@ Returns **[Feature][3]<[Polygon][1]>** Masked Polygon (exterior ring with holes)

[4]: https://tools.ietf.org/html/rfc7946#section-3.3

[5]: https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Object

[6]: https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Boolean

<!-- This file is automatically generated. Please don't edit it directly. If you find an error, edit the source file of the module in question (likely index.js or index.ts), and re-run "yarn docs" from the root of the turf project. -->

---
Expand Down
44 changes: 37 additions & 7 deletions packages/turf-mask/bench.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,21 @@
import { Feature, FeatureCollection, Polygon, MultiPolygon } from "geojson";
import { FeatureCollection, Polygon, MultiPolygon, Feature } from "geojson";
import fs from "fs";
import path from "path";
import { fileURLToPath } from "url";
import { loadJsonFileSync } from "load-json-file";
import Benchmark, { Event } from "benchmark";
import { mask as turfMask } from "./index.js";
import clone from "@turf/clone";

// type guard to narrow the type of the fixtures
const isPolygonFeature = (
feature: Feature<Polygon | MultiPolygon>
): feature is Feature<Polygon> => feature.geometry.type === "Polygon";

const __dirname = path.dirname(fileURLToPath(import.meta.url));

const SKIP = ["multi-polygon.geojson", "overlapping.geojson"];

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do these still need to be skipped?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah they fail the 'Fixtures should have two features' check 🤦

const suite = new Benchmark.Suite("turf-mask");

const directories = {
Expand All @@ -17,22 +25,44 @@ const directories = {

let fixtures = fs.readdirSync(directories.in).map((filename) => {
return {
filename,
name: path.parse(filename).name,
geojson: loadJsonFileSync(
path.join(directories.in, filename)
) as FeatureCollection<Polygon | MultiPolygon>,
};
});

for (const { name, geojson } of fixtures) {
for (const { name, filename, geojson } of fixtures) {
if (SKIP.includes(filename)) continue;

const [polygon, masking] = geojson.features;
suite.add(name, () => turfMask(polygon, masking as Feature<Polygon>));
if (!masking || !isPolygonFeature(masking)) {
throw new Error(
"Fixtures should have two features: an input feature and a Polygon masking feature."
);
}

const getSuite = ({ mutate }: { mutate: boolean }) => ({
name: `${name} (mutate = ${mutate})`,
fn: () => {
// We clone the inputs to prevent tests from affecting each other
turfMask(clone(polygon), clone(masking), { mutate });
},
});

suite.add(getSuite({ mutate: false }));
suite.add(getSuite({ mutate: true }));
}

// basic x 4,627 ops/sec ±25.23% (21 runs sampled)
// mask-outside x 3,892 ops/sec ±34.80% (15 runs sampled)
// multi-polygon x 5,837 ops/sec ±3.03% (91 runs sampled)
// overlapping x 22,326 ops/sec ±1.34% (90 runs sampled)
/**
* Benchmark Results:
*
* basic (mutate = false) x 294,373 ops/sec ±0.25% (95 runs sampled)
* basic (mutate = true) x 307,397 ops/sec ±0.13% (97 runs sampled)
* mask-outside (mutate = false) x 100,575 ops/sec ±0.55% (97 runs sampled)
* mask-outside (mutate = true) x 103,180 ops/sec ±0.40% (94 runs sampled)
*/
suite
.on("cycle", (event: Event) => {
console.log(String(event.target));
Expand Down
20 changes: 16 additions & 4 deletions packages/turf-mask/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
} from "geojson";
import { polygon as createPolygon, multiPolygon } from "@turf/helpers";
import polygonClipping, { Geom } from "polygon-clipping";
import { clone } from "@turf/clone";

/**
* Takes polygons or multipolygons and an optional mask, and returns an exterior
Expand All @@ -15,7 +16,9 @@ import polygonClipping, { Geom } from "polygon-clipping";
* @name mask
* @param {Polygon|MultiPolygon|Feature<Polygon|MultiPolygon>|FeatureCollection<Polygon|MultiPolygon>} polygon GeoJSON polygon used as interior rings or holes
* @param {Polygon|Feature<Polygon>} [mask] GeoJSON polygon used as the exterior ring (if undefined, the world extent is used)
* @returns {Feature<Polygon>} Masked Polygon (exterior ring with holes).
* @param {Object} [options={}] Optional parameters
* @param {boolean} [options.mutate=false] allows the `mask` GeoJSON input to be mutated (performance improvement if true)
* @returns {Feature<Polygon>} Masked Polygon (exterior ring with holes)
* @example
* const polygon = turf.polygon([[[112, -21], [116, -36], [146, -39], [153, -24], [133, -10], [112, -21]]]);
* const mask = turf.polygon([[[90, -55], [170, -55], [170, 10], [90, 10], [90, -55]]]);
Expand All @@ -27,10 +30,19 @@ import polygonClipping, { Geom } from "polygon-clipping";
*/
function mask<T extends Polygon | MultiPolygon>(
polygon: T | Feature<T> | FeatureCollection<T>,
mask?: Polygon | Feature<Polygon>
mask?: Polygon | Feature<Polygon>,
options?: { mutate?: boolean }
): Feature<Polygon> {
// Define mask
const maskPolygon = createMask(mask);
const mutate = options?.mutate ?? false; // by default, do not mutate

let maskTemplate = mask;
if (mask && mutate === false) {
// Clone mask if requested to avoid side effects
maskTemplate = clone(mask);
}
smallsaucepan marked this conversation as resolved.
Show resolved Hide resolved

// Define initial mask
const maskPolygon = createMask(maskTemplate);

let polygonOuters = null;
if (polygon.type === "FeatureCollection") {
Expand Down
1 change: 1 addition & 0 deletions packages/turf-mask/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
"write-json-file": "^5.0.0"
},
"dependencies": {
"@turf/clone": "workspace:^",
"@turf/helpers": "workspace:^",
"@types/geojson": "7946.0.8",
"polygon-clipping": "^0.15.3",
Expand Down
31 changes: 31 additions & 0 deletions packages/turf-mask/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { fileURLToPath } from "url";
import { loadJsonFileSync } from "load-json-file";
import { writeJsonFileSync } from "write-json-file";
import { mask } from "./index.js";
import { clone } from "@turf/clone";

const __dirname = path.dirname(fileURLToPath(import.meta.url));

Expand Down Expand Up @@ -48,6 +49,36 @@ test("turf-mask", (t) => {
t.end();
});

const getBasicPolygonAndMask = () => {
const basicFixture = fixtures.find(
({ filename }) => filename === "basic.geojson"
);
if (!basicFixture) throw new Error("basic.geojson not found");
return basicFixture.geojson.features;
};

test("turf-mask -- doesn't mutate inputs by default", (t) => {
const [polygon, masking] = getBasicPolygonAndMask();
const maskClone = clone(masking);

mask(polygon, masking);

t.deepEquals(masking, maskClone, "mask input should not be mutated");

t.end();
});

test("turf-mask -- mutates mask input when mutate = true", (t) => {
const [polygon, masking] = getBasicPolygonAndMask();
const maskClone = clone(masking);

mask(polygon, masking, { mutate: true });

t.notDeepEqual(masking, maskClone, "mask input should be mutated");

t.end();
});

test("turf-mask polygon geometry", (t) => {
// A polygon somewhere
const polyCoords: Position[] = [
Expand Down
1 change: 1 addition & 0 deletions packages/turf-mask/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,4 @@ const poly2 = polygon([

mask(poly1);
mask(poly1, poly2);
mask(poly1, poly2, { mutate: true });
3 changes: 3 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading