koopjs / koop

Transform, query, and download geospatial data on the web.
http://koopjs.github.io
Other
659 stars 127 forks source link

suppress metadata/feature property warnings or test for NaN? #330

Closed mpayson closed 6 years ago

mpayson commented 6 years ago

I'm working with a dataset that has a good number of missing values. This causes Koop to throw

WARNING: requested provider's metadata field... not found in feature properties

on every query. Is there a way to silence or modify these warnings?

A couple examples:

Since the metadata/feature property check is okay with featureField.type = Integer and field.type = Date | Double, my warnings could be fixed if detectType returns Integer for NaN, but I'm not sure if this would have unintended side effects.

function detectType (value) {
  var type = typeof value

  if (type === 'number') {
    return Number.isInteger(value) || isNaN(value) ? 'Integer' : 'Double'
  } // ...
}

thanks!

rgwozdz commented 6 years ago

@mpayson thanks for reporting this. Seems like at a minimum these warnings should only be issued when NODE_ENV !== 'production'. Would that be enough for your use case? We put in the warnings because ArcGIS clients can break when the fields array elements have data types that do not match the data types in the feature properties.

I think the only unintended consequence of your proposal is that when there is a mismatch between the fields data-type and feature properties data type, and that mismatch causes breakage on ArcGIS client, a developer wouldn't get the warning as a clue for diagnosing the problem.

mpayson commented 6 years ago

@rgwozdz makes sense, that would fit my use case, thanks!

rgwozdz commented 6 years ago

@mpayson FYI, just back from holiday leave. This is still on my radar.

rgwozdz commented 6 years ago

@mpayson - FeatureServer release 2.15.2 will suppress warnings when NODE_ENV === 'production' or KOOP_WARNINGS === 'suppress'. yarn upgrade should get you this latest version of FeatureServer.

rgwozdz commented 6 years ago

There are still a few warnings in the Winnow dependency. Will address those in same manner.

rgwozdz commented 6 years ago

Added warning suppression to Winnow 1.16.4.