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

Set centered param default to True #836

Merged
merged 2 commits into from
Jul 12, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 1 addition & 1 deletion packages/turf-point-grid/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ Creates a [Point](http://geojson.org/geojson-spec.html#point) grid from a boundi
- `bbox` **([Array](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array)<[number](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number)> | [FeatureCollection](http://geojson.org/geojson-spec.html#feature-collection-objects) \| [Feature](http://geojson.org/geojson-spec.html#feature-objects)<any>)** extent in [minX, minY, maxX, maxY] order
- `cellSide` **[number](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number)** the distance between points
- `units` **\[[string](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String)]** used in calculating cellSide, can be degrees, radians, miles, or kilometers (optional, default `kilometers`)
- `centered` **\[[boolean](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Boolean)]** adjust points position to center the grid into bbox (optional, default `false`)
- `centered` **\[[boolean](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Boolean)]** adjust points position to center the grid into bbox (optional, default `true`). **This parameter is going to be removed** in the next major release, having the output always centered into bbox.
- `bboxIsMask` **\[[boolean](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Boolean)]** if true, and bbox is a Polygon or MultiPolygon, the grid Point will be created
only if inside the bbox Polygon(s) (optional, default `false`)

Expand Down
2 changes: 1 addition & 1 deletion packages/turf-point-grid/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@ import {BBox, Points, Units, Feature, Features} from '@turf/helpers';
/**
* http://turfjs.org/docs/#pointgrid
*/
declare function pointGrid(bbox: BBox | Feature<any> | Features<any>, cellSide: number, units?: Units, centered?: boolean, bboxIsMask?: boolean): Points;
declare function pointGrid(bbox: BBox | Feature<any> | Array<number>, cellSide: number, units?: Units, centered?: boolean, bboxIsMask?: boolean): Points;
Copy link
Member

@DenisCarriere DenisCarriere Jul 12, 2017

Choose a reason for hiding this comment

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

  1. Why the removal of Features<any> this is to support FeatureCollection, I guess we should use the full name of the Type (FeatureCollection<any>) instead of making the types plural.

  2. BBox is already defined - Array<number> is not needed

BBox => [number, number, number, number]

CC: @stebogit

Copy link
Member

@DenisCarriere DenisCarriere Jul 12, 2017

Choose a reason for hiding this comment

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

I need to take some time and update all the Typescript definitions for each module (all 85 of them.. 😱) make them all consistent.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@DenisCarriere it'd be nice if we had some kind of types file (TYPES.md?) maybe in the root as reference for all Turf modules. With that I could help updating all the modules and would be useful, when creating new models, for who like me doesn't know typescript

Copy link
Member

Choose a reason for hiding this comment

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

That would be a good idea, maybe I'll create that before updating all the definitions.

My approach to Typescript is not as strict as others like DefinitelyTyped which enforce strict linting (don't think we are there yet for TurfJS - might add a basic TS linting config).

declare namespace pointGrid { }
export = pointGrid;
8 changes: 4 additions & 4 deletions packages/turf-point-grid/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ var featureCollection = helpers.featureCollection;
* @param {Array<number>|FeatureCollection|Feature<any>} bbox extent in [minX, minY, maxX, maxY] order
* @param {number} cellSide the distance between points
* @param {string} [units=kilometers] used in calculating cellSide, can be degrees, radians, miles, or kilometers
* @param {boolean} [centered=false] adjust points position to center the grid into bbox
* @param {boolean} [centered=true] adjust points position to center the grid into bbox. **This parameter is going to be removed** in the next major release, having the output always centered into bbox.
* @param {boolean} [bboxIsMask=false] if true, and bbox is a Polygon or MultiPolygon, the grid Point will be created
* only if inside the bbox Polygon(s)
* @returns {FeatureCollection<Point>} grid of points
Expand Down Expand Up @@ -47,7 +47,7 @@ module.exports = function (bbox, cellSide, units, centered, bboxIsMask) {
var yFraction = cellSide / (distance(point([west, south]), point([west, north]), units));
var cellHeight = yFraction * (north - south);

if (centered === true) {
if (centered !== false) {
var bboxHorizontalSide = (east - west);
var bboxVerticalSide = (north - south);
var columns = Math.floor(bboxHorizontalSide / cellWidth);
Expand All @@ -60,10 +60,10 @@ module.exports = function (bbox, cellSide, units, centered, bboxIsMask) {
var isPoly = !Array.isArray(bboxMask) && (getGeomType(bboxMask) === 'Polygon' || getGeomType(bboxMask) === 'MultiPolygon');

var currentX = west;
if (centered === true) currentX += deltaX;
if (centered !== false) currentX += deltaX;
while (currentX <= east) {
var currentY = south;
if (centered === true) currentY += deltaY;
if (centered !== false) currentY += deltaY;
while (currentY <= north) {
var pt = point([currentX, currentY]);
if (bboxIsMask === true && isPoly) {
Expand Down
2 changes: 0 additions & 2 deletions packages/turf-point-grid/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,8 @@
},
"homepage": "https:/Turfjs/turf",
"devDependencies": {
"@turf/meta": "^4.5.2",
"benchmark": "^2.1.4",
"load-json-file": "^2.0.0",
"mkdirp": "^0.5.1",
"tape": "^4.6.3",
"write-json-file": "^2.0.0"
},
Expand Down
46 changes: 14 additions & 32 deletions packages/turf-point-grid/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,7 @@ const test = require('tape');
const path = require('path');
const load = require('load-json-file');
const write = require('write-json-file');
const mkdirp = require('mkdirp');
const {featureEach} = require('@turf/meta');
const grid = require('./');
const pointGrid = require('./');

const directories = {
in: path.join(__dirname, 'test', 'in') + path.sep,
Expand All @@ -23,41 +21,25 @@ let fixtures = fs.readdirSync(directories.in).map(filename => {

test('turf-point-grid', t => {
for (const {name, geojson} of fixtures) {
const grid5 = grid(geojson, 5, 'miles');
const grid20 = grid(geojson, 20, 'miles');
const gridCentered = grid(geojson, 12.5, 'miles', true);
const gridMasked = grid(geojson, 15, 'miles', true, true);
const {cellSide, units, centered, bboxIsMask} = geojson.properties;
const result = pointGrid(geojson, cellSide, units, centered, bboxIsMask);

// Add current GeoJSON to grid results
featureEach(geojson, feature => {
feature.properties = {
stroke: '#F00',
'stroke-width': 6,
'fill-opacity': 0
};
grid5.features.push(feature);
grid20.features.push(feature);
gridCentered.features.push(feature);
gridMasked.features.push(feature);
});
// Add styled GeoJSON to the result
geojson.properties = {
stroke: '#F00',
'stroke-width': 6,
'fill-opacity': 0
};
result.features.push(geojson);

if (process.env.REGEN) {
mkdirp.sync(directories.out + name);
write.sync(path.join(directories.out, name, '5-miles.geojson'), grid5);
write.sync(path.join(directories.out, name, '20-miles.geojson'), grid20);
write.sync(path.join(directories.out, name, 'centered.geojson'), gridCentered);
write.sync(path.join(directories.out, name, 'masked.geojson'), gridMasked);
}
t.deepEqual(grid5, load.sync(path.join(directories.out, name, '5-miles.geojson')), name + ' -- 5 miles');
t.deepEqual(grid20, load.sync(path.join(directories.out, name, '20-miles.geojson')), name + ' -- 20 miles');
t.deepEqual(gridCentered, load.sync(path.join(directories.out, name, 'centered.geojson')), name + ' -- centered');
t.deepEqual(gridMasked, load.sync(path.join(directories.out, name, 'masked.geojson')), name + ' -- masked');
if (process.env.REGEN) write.sync(directories.out + name + '.geojson', result);
t.deepEqual(result, load.sync(directories.out + name + '.geojson'), name);
}
t.end();
});

test('turf-point-grid -- throws', t => {
t.throws(() => grid(null, 10), /bbox is required/, 'missing bbox');
t.throws(() => grid([-95, 30, 40], 10), /bbox must contain 4 numbers/, 'invalid bbox');
t.throws(() => pointGrid(null, 10), /bbox is required/, 'missing bbox');
t.throws(() => pointGrid([-95, 30, 40], 10), /bbox must contain 4 numbers/, 'invalid bbox');
t.end();
});
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
{
"type": "Feature",
"properties": {},
"properties": {
"cellSide": 10,
"units": "miles"
},
"geometry": {
"type": "Polygon",
"coordinates": [
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
{
"type": "Feature",
"properties": {},
"properties": {
"cellSide": 20,
"units": "miles"
},
"geometry": {
"type": "Polygon",
"coordinates": [
Expand Down
34 changes: 34 additions & 0 deletions packages/turf-point-grid/test/in/not-centered.geojson
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
{
"type": "Feature",
"properties": {
"cellSide": 1,
"centered": false
},
"geometry": {
"type": "Polygon",
"coordinates": [
[
[
-72.00851440429688,
-13.575913832365224
],
[
-71.90826416015625,
-13.575913832365224
],
[
-71.90826416015625,
-13.486791161568776
],
[
-72.00851440429688,
-13.486791161568776
],
[
-72.00851440429688,
-13.575913832365224
]
]
]
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
{
"type": "Feature",
"properties": {},
"properties": {
"cellSide": 12.5,
"units": "miles",
"bboxIsMask": true
},
"geometry": {
"type": "Polygon",
"coordinates": [
Expand Down
5 changes: 4 additions & 1 deletion packages/turf-point-grid/test/in/resolute.geojson
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
{
"type": "Feature",
"properties": {},
"properties": {
"cellSide": 12.5,
"units": "miles"
},
"geometry": {
"type": "Polygon",
"coordinates": [
Expand Down
Loading