humangeo / leaflet-dvf

Leaflet Data Visualization Framework
MIT License
689 stars 152 forks source link

when the center of a geohash's bounding box is 0,0 dataLaer fails -- 0,0 is a valid lat/lng #115

Open tlhampton13 opened 7 years ago

tlhampton13 commented 7 years ago

In leaflet.dvf.datalayer.js GEOHASH function.

The following condition fails if the center of the bounding box evaluates to 0,0 witch is a valid lat/lng location. Specifically, this is the location that the Equator and the Prime Meridian intersect.

if (locationInfo.latitude[2] && locationInfo.longitude[2]) {
    bounds = new L.LatLngBounds(new L.LatLng(locationInfo.latitude[0], locationInfo.longitude[0]), new L.LatLng(locationInfo.latitude[1], locationInfo.longitude[1]));
}

a little farther down, since the bounds is undefined this line fails

center: bounds.getCenter()

This will cause an entire dataLayer to fail if a single center point is 0,0.

tlhampton13 commented 7 years ago

Actually, this will fail anytime the center of a geohash falls on the Equator or the Prime Meridian.

sfairgrieve commented 7 years ago

@tlhampton13 good catch. Should be fixed soon.

sfairgrieve commented 7 years ago

Giving this some more thought, I don't believe this scenario will ever happen, or is it actually happening for you? The geohash of 0, 0 is s0, s00, s000, s0000, and so on. If you calculate the center of any of those geohashes, it's never 0, 0. In order for it to actually be 0, 0, you'd have to have a super long geohash string. In general, that code could probably use some refactoring/cleanup, but I don't believe it will fail with geohashes of realistic length. @tlhampton13 What do you think?

tlhampton13 commented 7 years ago

@sfairgrieve

You are correct. No geohashes actually span the Equator or Prime Meridian. These lines would fall along a bounding edge of the adjacent geohashes.

I am getting a failure in the GEOHASH function though because the bounds is undefined. But the reason is not as I originally described.

In some of my records the geohash fields is empty string.

{
  "geohashField": ""
}

When this happens the GeoHash.decodeGeoHash() method actually returns location information for the empty string. Notice that the third element in the arrays (center point) is missing.

{
  "latitude": [-90.0, 90.0],
  "longitude": [-180.0, 180.0] 
}

then the if condition fails because the center point is not present.

if (locationInfo.latitude[2] && locationInfo.longitude[2])

I can change the data so that the geohashField is not present if it is "", but then records without a field fail. So I add a filter to the dataLayer, but that fails because the _loadRecords method lookups the geocode before checking includeLayer value.

sfairgrieve commented 7 years ago

@tlhampton13 apologies for the delayed response. Did you make any progress with this? For now, my suggestion would be to either remove records that have a blank geohashField before passing the data to the DataLayer (e.g. using lodash or your own JavaScript) or implementing your own LocationMode function (you can use the GEOHASH one as a starting point). I agree that the existing code in Leaflet DVF needs to be updated to be more resilient to cases like this or provide more ways of preventing it.