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

🛠 Extend Circle Options #2516

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Empty file modified packages/turf-boolean-clockwise/README.md
100755 → 100644
Empty file.
Empty file modified packages/turf-boolean-clockwise/bench.js
100755 → 100644
Empty file.
Empty file modified packages/turf-boolean-clockwise/package.json
100755 → 100644
Empty file.
Empty file modified packages/turf-boolean-overlap/README.md
100755 → 100644
Empty file.
Empty file modified packages/turf-boolean-overlap/bench.js
100755 → 100644
Empty file.
Empty file modified packages/turf-boolean-overlap/package.json
100755 → 100644
Empty file.
42 changes: 31 additions & 11 deletions packages/turf-circle/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { GeoJsonProperties, Feature, Point, Polygon } from "geojson";
import { GeoJsonProperties, Feature, Point, Polygon, BBox } from "geojson";
import destination from "@turf/destination";
import { polygon, Units } from "@turf/helpers";
import { Id, polygon, Units } from "@turf/helpers";

/**
* Takes a {@link Point} and calculates the circle polygon given a radius in degrees, radians, miles, or kilometers; and steps for precision.
Expand All @@ -12,6 +12,8 @@ import { polygon, Units } from "@turf/helpers";
* @param {number} [options.steps=64] number of steps
* @param {string} [options.units='kilometers'] miles, kilometers, degrees, or radians
* @param {Object} [options.properties={}] properties
* @param {Array<number>} [options.bbox] Bounding Box Array [west, south, east, north] associated with the Feature
* @param {string|number} [options.id] Identifier associated with the Feature
* @returns {Feature<Polygon>} circle polygon
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can these please clarify these relate to the result Feature, for example "Identifier to assign to the resulting circle feature", "Bounding Box Array [west, south, east, north] to assign to the resulting circle Feature"

Copy link
Collaborator

@twelch twelch Nov 14, 2023

Choose a reason for hiding this comment

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

Can the center param typedoc also please clarify that if it is a Feature, then its properties/id will be assigned to the resulting circle feature, and whether they will have precedence over options passed.

* @example
* var center = [-75.343, 39.984];
Expand All @@ -29,27 +31,45 @@ function circle<P extends GeoJsonProperties = GeoJsonProperties>(
steps?: number;
units?: Units;
properties?: P;
bbox?: BBox;
id?: Id;
} = {}
): Feature<Polygon, P> {
// default params
const steps = options.steps || 64;
const properties: any = options.properties
? options.properties
: !Array.isArray(center) && center.type === "Feature" && center.properties
? center.properties
: {};
const stepAngle = -360 / steps;

// main
let properties: P | undefined = options.properties;
let bboxValue: BBox | undefined;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not essential but consider letting the types flow through without manual duplication like

let properties: Pick<typeof options, 'properties'> = options.properties;

let idValue: Id | undefined;

if (!Array.isArray(center) && center.type === "Feature") {
properties = properties || center.properties;
bboxValue = center.bbox;
idValue = center.id;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you think it makes sense to pass on the bbox of the center point to polygon()? I would suspect that the bbox of the point will almost never be representative of the bbox of the resulting circle. I think it would be better to leave it undefined if the caller doesn't provide their own options.bbox. And the user can generate their own using bbox()

}

bboxValue = bboxValue || options.bbox;
idValue = idValue || options.id;

let _options: { bbox?: BBox; id?: Id } | undefined;
if (bboxValue || idValue) {
_options = {
...(bboxValue ? { bbox: bboxValue } : {}),
...(idValue ? { id: idValue } : {}),
};
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

This feels a bit overcomplicated. Can it be removed altogether and you can construct the options object in the call to polygon()? For example:

return polygon([coordinates], properties, {bbox: bboxValue, id: idValue);

// Calculate circle coordinates
const coordinates = [];
for (let i = 0; i < steps; i++) {
coordinates.push(
destination(center, radius, (i * -360) / steps, options).geometry
.coordinates
destination(center, radius, i * stepAngle, options).geometry.coordinates
);
}
coordinates.push(coordinates[0]);

return polygon([coordinates], properties);
return polygon([coordinates], properties, _options);
}

export default circle;
Empty file modified scripts/add-import-extensions.js
100755 → 100644
Empty file.
Empty file modified scripts/check-dependencies.js
100755 → 100644
Empty file.
Empty file modified scripts/create-new-module
100755 → 100644
Empty file.
Empty file modified scripts/generate-readmes.ts
100755 → 100644
Empty file.
Empty file modified scripts/list-modules-to-markdown
100755 → 100644
Empty file.
Empty file modified scripts/npm-publish-all
100755 → 100644
Empty file.
Empty file modified scripts/organization-make-public
100755 → 100644
Empty file.
Empty file modified scripts/update-dependencies
100755 → 100644
Empty file.
Loading