Turf: Type Checking for Coordinates and NaN

[mark-meyer]

Creating points doesn't coerce strings into numbers but instead gives an error:

var lat = '61',
var lon = '-149'
turf.point([lon, lat])
// Error: Coordinates must contain numbers

That's a little unjavascript-like, but reasonable enough. A common way of dealing with this is to coerce numbers with parseInt() or:

turf.point([+lon, +lat])

This works, but creates a false sense of security. Because turf's error checking depends on typeof,
this works with no error:

var lat = 'Foo',
var lon = 'Bar'
turf.point([+lon, +lat])

+"Foo" returns NaN which is typeof number. I wonder if it would be possible for turf to fail here when given NaN to make it easier to pass in converted strings without type checking. If Turf is going to do type checking, it would be great if it was a little more thorough.

[stebogit]

@DenisCarriere I noticed you actually removed that check here (@markmeyerphoto this change is not in the current/latest release, as you can see).
If we actually had the type validation on point coordinates I guess we should have it in all the similar helpers functions, which would probably slow down particularly the multi feature creation functions. Is this why you removed it?

If we did want to have a reliable check for numbers, I tested this function: https://repl.it/KXcu/4

[DenisCarriere]

I noticed you actually removed that check

👍 Looks like it was removed by accident 🤦‍♂️ , that was a big commit and I might of missed that when I committed that change.

As for more advanced type checking, we shouldn't be doing anything more than the basic typeof validation checks.

If one wants advanced Type checking then use Typescript which will handle all your type checking.

we should have it in all the similar helpers functions

~Points should be the only helper function to do this~, all the other helper functions would be overkill and would significantly slow down the performance if the geometry is very large. (Edit: We could test other helper methods, but only for the 1st point in the geometry.)

I would recommend we stick with the current style of basic validation:

• typeof geojson === string
• geojson === undefined
• geojson === null

[DenisCarriere]

@stebogit 👍 Sweet method! 👏 Maybe we can add that to @turf/helpers?

function isNumber(x) {
    return !isNaN(x) && x !== null && !Array.isArray(x);
}

[DenisCarriere]

To-Do list

• [ ] include isNumber to @turf/helpers
• [ ] add type checking for helpers.point using isNumber

[stebogit]

It should be added to JavaScript!!! 😆

[DenisCarriere]

HAHA Agreed! SOOO many things should be added to Javascript. Today I was annoyed that I couldn't do Object.omit I had to use lodash.omit. There's lots of things are missing in JS.

[barbalex]

could we have this update on npm too please?

npm remains on version 4.6.0 (that does not exist on github???)

[DenisCarriere]

@barbalex very strange on NPM, the newest version was able to be downloaded using npm install @turf/[email protected], however whenever you installed it via npm install @turf/helpers it would default to v4.6.0.

This was the same with all the Turf modules, I republished all the TurfJS modules, isNumber should be available now in v4.7.3.

[strech345]

I would like the possebility to disable this check. Because for me it look like very cpu expensive, and i don't need this check if i have prepared coordinates. For me the performance is very important.

[rowanwins]

I agree with @strech345 on this one. I think one of our guiding principles in Turf is that people pass in valid geojson. I think we've made that call elsewhere not to handle errors due to invalid inputs.

I'm not sure if @strech345 suggestion of adding an option is viable just because of the sheer number of entry points so I'd be in favour of removing the checks.

But I wonder if we could have a makeValid module which performs a range of tasks including ensuring valid coord types. We could start with some basics in this module and slowly include more functionality in to it. We could have an options object

makeValid(someFeatureCollection, { checkCoorindateTypes: true, unkink: true })
Something similar to this repairGeometry function in Esri world

[strech345]

@rowanwins yes u right, a makeValid mudule would be make sense.

[stebogit]

👍 @rowanwins we can definitely assume/require Turf inputs to be valid GeoJSON, and we might want to mention it in the docs to make it official.

A validating tool has always been needed though 😄
To make it easier, at least as a first step, we might provide a simpler (boolean) validity checker, instead of a module that could also correct an invalid feature.

[DenisCarriere]

1. ✅ Having a validating module with multiple methods would definitely be a useful. I've added a few validating methods already in @turf/helpers:

   • validateBBox
   • validateId

2. 🚀 I've improved the performance of invariant.getCoords() by 5x (PR https://github.com/Turfjs/turf/pull/1197).