From 1e14b9232dfe85f91e1df08d8aec3b12a74269a0 Mon Sep 17 00:00:00 2001
From: Marcus Needham <marcus@alltrails.com>
Date: Tue, 17 Nov 2015 15:26:36 -0800
Subject: [PATCH] Switch lat/long for Mapbox; Test to catch this; :dataset
option for Mapbox
---
README.md | 3 ++-
lib/geocoder/lookups/mapbox.rb | 12 +++++++++---
lib/geocoder/results/mapbox.rb | 4 ++--
test/unit/lookups/mapbox_test.rb | 2 +-
test/unit/result_test.rb | 24 ++++++++++++++++++++++++
5 files changed, 38 insertions(+), 7 deletions(-)
diff --git a/README.md b/README.md
index 98e98807..aedb2766 100644
--- a/README.md
+++ b/README.md
@@ -509,6 +509,7 @@ The [Google Places Details API](https://developers.google.com/places/documentati
#### Mapbox (`:mapbox`)
* **API key**: required
+* **Dataset**: Uses `mapbox.places` dataset by default. Specific the `mapbox.places-permanent` dataset by setting: `Geocoder.configure(:mapbox => {:dataset => "mapbox.places-permanent"})`
* **Key signup**: https://www.mapbox.com/pricing/
* **Quota**: depends on plan
* **Region**: complete coverage of US and Canada, partial coverage elsewhere (see for details: https://www.mapbox.com/developers/api/geocoding/#coverage)
@@ -516,7 +517,7 @@ The [Google Places Details API](https://developers.google.com/places/documentati
* **Languages**: English
* **Documentation**: https://www.mapbox.com/developers/api/geocoding/
* **Terms of Service**: https://www.mapbox.com/tos/
-* **Limitations**: Must be displayed on a Mapbox map. Cache results for up to 30 days.
+* **Limitations**: For `mapbox.places` dataset, must be displayed on a Mapbox map; Cache results for up to 30 days. For `mapbox.places-permanent` dataset, depends on plan.
* **Notes**: Currently in public beta.
#### Mapquest (`:mapquest`)
diff --git a/lib/geocoder/lookups/mapbox.rb b/lib/geocoder/lookups/mapbox.rb
index 8fa84884..797c2648 100644
--- a/lib/geocoder/lookups/mapbox.rb
+++ b/lib/geocoder/lookups/mapbox.rb
@@ -27,11 +27,16 @@ module Geocoder::Lookup
def url_query_string(query)
require 'cgi' unless defined?(CGI) && defined?(CGI.escape)
- CGI.escape query.text.to_s
+ if query.reverse_geocode?
+ lat,lon = query.coordinates
+ "#{CGI.escape lon},#{CGI.escape lat}"
+ else
+ CGI.escape query.text.to_s
+ end
end
def dataset
- "mapbox.places"
+ configuration[:dataset] || "mapbox.places"
end
def supported_protocols
@@ -39,8 +44,9 @@ module Geocoder::Lookup
end
def sort_relevant_feature(features)
+ # Sort by descending relevance; Favor original order for equal relevance (eg occurs for reverse geocoding)
features.sort_by do |feature|
- feature["relevance"]
+ [feature["relevance"],-features.index(feature)]
end.reverse
end
end
diff --git a/lib/geocoder/results/mapbox.rb b/lib/geocoder/results/mapbox.rb
index a948e753..9142e190 100644
--- a/lib/geocoder/results/mapbox.rb
+++ b/lib/geocoder/results/mapbox.rb
@@ -4,11 +4,11 @@ module Geocoder::Result
class Mapbox < Base
def latitude
- @latitude ||= @data["geometry"]["coordinates"].first.to_f
+ @latitude ||= @data["geometry"]["coordinates"].last.to_f
end
def longitude
- @longitude ||= @data["geometry"]["coordinates"].last.to_f
+ @longitude ||= @data["geometry"]["coordinates"].first.to_f
end
def coordinates
diff --git a/test/unit/lookups/mapbox_test.rb b/test/unit/lookups/mapbox_test.rb
index 688722d9..33d15924 100644
--- a/test/unit/lookups/mapbox_test.rb
+++ b/test/unit/lookups/mapbox_test.rb
@@ -16,7 +16,7 @@ class MapboxTest < GeocoderTestCase
def test_result_components
result = Geocoder.search("Madison Square Garden, New York, NY").first
- assert_equal [-73.991566, 40.749688], result.coordinates
+ assert_equal [40.749688, -73.991566], result.coordinates
assert_equal "Madison Square Garden", result.place_name
assert_equal "4 Penn Plz", result.street
assert_equal "New York", result.city
diff --git a/test/unit/result_test.rb b/test/unit/result_test.rb
index 132e85bd..886f5dd5 100644
--- a/test/unit/result_test.rb
+++ b/test/unit/result_test.rb
@@ -12,6 +12,30 @@ class ResultTest < GeocoderTestCase
end
end
+ def test_result_has_coords_in_reasonable_range_for_madison_square_garden
+ Geocoder::Lookup.street_services.each do |l|
+ next unless File.exist?(File.join("test", "fixtures", "#{l.to_s}_madison_square_garden"))
+ Geocoder.configure(:lookup => l)
+ set_api_key!(l)
+ result = Geocoder.search("Madison Square Garden, New York, NY 10001, United States").first
+ assert (result.latitude > 40 and result.latitude < 41), "Lookup #{l} latitude out of range"
+ assert (result.longitude > -74 and result.longitude < -73), "Lookup #{l} longitude out of range"
+ end
+ end
+
+ def test_result_accepts_reverse_coords_in_reasonable_range_for_madison_square_garden
+ Geocoder::Lookup.street_services.each do |l|
+ next unless File.exist?(File.join("test", "fixtures", "#{l.to_s}_madison_square_garden"))
+ next if [:bing, :esri, :geocoder_ca, :geocoder_us].include? l # Reverse fixture does not match forward
+ Geocoder.configure(:lookup => l)
+ set_api_key!(l)
+ result = Geocoder.search([40.750354, -73.993371]).first
+ assert (["New York", "New York City"].include? result.city), "Reverse lookup #{l} City does not match"
+ assert (result.latitude > 40 and result.latitude < 41), "Reverse lookup #{l} latitude out of range"
+ assert (result.longitude > -74 and result.longitude < -73), "Reverse lookup #{l} longitude out of range"
+ end
+ end
+
def test_yandex_result_without_city_does_not_raise_exception
assert_nothing_raised do
Geocoder.configure(:lookup => :yandex)
--
GitLab