bbox-polygon support 6 position BBox

[md8n]

bbox-polygon currently only support 4 position (2D) BBox values, although 6 position (3D) BBox values are obviously valid BBoxes.

It's trivial to support translating a 6 position BBox to a (2D) Polygon.
I've already produced an update to index.js and test.js in turf-bbox-polygon that supports both 4 and 6 position (2D & 3D) BBox values to 2D Polygons.

Optionally (pardon the pun) a member could be added to options as a flag to convert a 3D BBox to a 3D Polygon (I haven't done this).

[rowanwins]

Gday @md8n

Yep I think I we'd be happy to support 3D bounding boxes, what do you think @DenisCarriere ?

My preference would be to see it added as an option defaulting to false given that 90% of users probably wouldn't need it, perhaps called something like computeZ?

If you can put in a pull request along those lines that would be great! Perhaps before you do we'll just get clarification from Denis.

Thanks,
Rowan

[md8n]

The code I have at the moment doesn't care if the supplied bounding box is 2D or 3D - it just works, and returns a 2D Polygon.
But, if someone wanted a 3D Polygon returned (more correctly a surface), then I've done that kind of thing before. Although interpolating elevation, where two different elevation values were supplied in the 3D BBox, requires making some assumptions that may not suit everyone.

[md8n]

Having some issues publishing a canary.
Thanks to Error: EPERM: operation not permitted on npm 5.4 on windows.

[DenisCarriere]

bbox-polygon currently only support 4 position (2D) BBox values, although 6 position (3D) BBox values are obviously valid BBoxes.

I'm 👍 if we do the following logic to @turf/bbox-polygon:

• Input BBox has 4 positions => 2D polygon
• Input BBox has 6 positions => 3D polygon

Someone just needs to write up a PR with that appropriate logic.

Note: Since we're most likely moving towards Typescript, might be worth adding two different types definitions for BBox (BBox2D "default" & BBox3D).

[md8n]

There are several possible permutations for translating BBox2d | BBox3d to Polygon2d | Polygon3d - which is why I agree with:

Input BBox has 4 positions => 2D polygon
Input BBox has 6 positions => 3D polygon

As regards the TypeScript definition for BBOx geojson.d.ts defines it as follows:

export type BBox2d = [number, number, number, number];
export type BBox3d = [number, number, number, number, number, number];
export type BBox = BBox2d | BBox3d;

Which is exactly what's required.
However, BBox3d => Polygon3d has a problem, Bbox3d describes a volume, whereas Polygon3d describes a surface. The easiest way to do the conversion, is to choose one of either lat or lon to be the 'slope'. I'd recommend this approach, with a convention as to which dimension is the default, and with an option to allow its selection.

I think it would be best to support BBox2d <=> BBox3d conversions as a separate function

• BBox3d => BBox2d (- minZ, maxZ)
• BBox2d + Elevation [] => BBox3d

The two values of Elevation [] would be used as the minZ and maxZ values within the BBox3d.

Elevation [] has the following objective applied to it:
Can it be reasonably coerced into an ordered array of two numeric values.
e.g.

• What is supplied is not parseable as a number or an [] of numbers => throw an exception

However, from experience, the following are reasonable coercions:

• undefined, null or empty [] => [0, 0]
• single value v => [v, v]
• single value [v] => [v, v]
• three or more values [a, b, ...] => [a, b]

And finally, the ordering

• two values a, b such that a > b => [b, a]
• two values a, b such that a <= b => [a, b]

[DenisCarriere]

FYI on the Typescript definition, the TurfJS geojson.d.ts is derived (minor fork) from:
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/geojson/index.d.ts#L31

So any changes that are made in TurfJS's geojson.d.ts, should also reflect upstream to DefinitelyTyped:

https://github.com/Turfjs/turf/blob/master/packages/turf-helpers/lib/geojson.d.ts#L46-L48

@md8n I support your train of thought 👍

[md8n]

https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/geojson/index.d.ts#L31
with its
export type BBox = [number, number, number, number] | [number, number, number, number, number, number];
definitely ain't as good as Turf's def

In the end we'd also want
export type Position = number[]; // [number, number] | [number, number, number];
to be

export type Position2d = [number, number];
export type Position3d = [number, number, number];
export type Position = Position2d | Position3d;

[DenisCarriere]

@md8n Agreed, there's lot of changes that I would suggest in the Typescript definition, however moving outside of number[] for Position would require all coordinates to be pre-defined, if set an Array of Numbers by default it will result in number[].

definitely ain't as good as Turf's def

I also like Turf's def (kinda biased because I made it... 😁 )

[md8n]

First of all let's make my Position suggestion a separate issue from the BBox 2d | 3d support.

... and with that - here's a discussion on it ...

however moving outside of number[] for Position would require all coordinates to be pre-defined, if set an Array of Numbers by default it will result in number[].

I'm assuming that probably the main reason why the existing geojson/index.d.ts defines it that why is either it provides deliberate backwards compatibility with the previous version of the GeoJSON spec, or (more likely) no one got around to changing it 😁.

My concern, is that the further we are away from the spec then the more chance there is for subtle errors creeping in, especially when in use. Leveraging TypeScript for what its good at, in this case defining some type as a union of other types, gets everything (because Position touches everything) a lot closer to the spec.