Merge pull request #90 from google/recovery-issue

Fix short code recovery near poles.

Author: drinckes

go/olc_test.go

@@ -45,6 +45,7 @@ type (
 		code     string
 		lat, lng float64
 		short    string
+		tType    string
 	}
 )
 
@@ -83,6 +84,7 @@ func init() {
 				code: string(cols[0]),
 				lat:  mustFloat(cols[1]), lng: mustFloat(cols[2]),
 				short: string(cols[3]),
+				tType: string(cols[4]),
 			})
 		}
 	}()
@@ -139,24 +141,28 @@ func TestDecode(t *testing.T) {
 
 func TestShorten(t *testing.T) {
 	for i, elt := range shorten {
-		got, err := Shorten(elt.code, elt.lat, elt.lng)
-		if err != nil {
-			t.Errorf("%d. shorten %q: %v", i, elt.code, err)
-			t.FailNow()
-		}
-		if got != elt.short {
-			t.Errorf("%d. shorten got %q, awaited %q.", i, got, elt.short)
-			t.FailNow()
+		if elt.tType == "B" || elt.tType == "S" {
+			got, err := Shorten(elt.code, elt.lat, elt.lng)
+			if err != nil {
+				t.Errorf("%d. shorten %q: %v", i, elt.code, err)
+				t.FailNow()
+			}
+			if got != elt.short {
+				t.Errorf("%d. shorten got %q, awaited %q.", i, got, elt.short)
+				t.FailNow()
+			}
 		}
 
-		got, err = RecoverNearest(got, elt.lat, elt.lng)
-		if err != nil {
-			t.Errorf("%d. nearest %q: %v", i, got, err)
-			t.FailNow()
-		}
-		if got != elt.code {
-			t.Errorf("%d. nearest got %q, awaited %q.", i, got, elt.code)
-			t.FailNow()
+		if elt.tType == "B" || elt.tType == "R" {
+			got, err := RecoverNearest(elt.short, elt.lat, elt.lng)
+			if err != nil {
+				t.Errorf("%d. nearest %q: %v", i, got, err)
+				t.FailNow()
+			}
+			if got != elt.code {
+				t.Errorf("%d. nearest got %q, awaited %q.", i, got, elt.code)
+				t.FailNow()
+			}
 		}
 	}
 }

go/shorten.go

@@ -114,7 +114,7 @@ func RecoverNearest(code string, lat, lng float64) (string, error) {
 	resolution := math.Pow(20, float64(2-(padLen/2)))
 
 	// Distance from the center to an edge (in degrees).
-	areaToEdge := float64(resolution) / 2
+	halfRes := float64(resolution) / 2
 
 	// Use the reference location to pad the supplied short code and decode it.
 	area, err := Decode(Encode(lat, lng, 0)[:padLen] + code)
@@ -123,24 +123,23 @@ func RecoverNearest(code string, lat, lng float64) (string, error) {
 	}
 
 	// How many degrees latitude is the code from the reference? If it is more
-	// than half the resolution, we need to move it east or west.
+	// than half the resolution, we need to move it south or north but keep it
+	// within -90 to 90 degrees.
 	centerLat, centerLng := area.Center()
-	degDiff := centerLat - lat
-	if degDiff > areaToEdge {
-		// If the center of the short code is more than half a cell east,
-		// then the best match will be one position west.
+	if lat + halfRes < centerLat && centerLat - resolution >= -latMax {
+		// If the proposed code is more than half a cell north of the reference location,
+		// it's too far, and the best match will be one cell south.
 		centerLat -= resolution
-	} else if degDiff < -areaToEdge {
-		// If the center of the short code is more than half a cell west,
-		// then the best match will be one position east.
+	} else if lat - halfRes > centerLat && centerLat + resolution <= latMax {
+		// If the proposed code is more than half a cell south of the reference location,
+		// it's too far, and the best match will be one cell north.
 		centerLat += resolution
 	}
 
 	// How many degrees longitude is the code from the reference?
-	degDiff = centerLng - lng
-	if degDiff > areaToEdge {
+	if lng + halfRes < centerLng {
 		centerLng -= resolution
-	} else if degDiff < -areaToEdge {
+	} else if lng - halfRes > centerLng {
 		centerLng += resolution
 	}
 

js/src/openlocationcode.js

@@ -384,30 +384,31 @@
     // The resolution (height and width) of the padded area in degrees.
     var resolution = Math.pow(20, 2 - (paddingLength / 2));
     // Distance from the center to an edge (in degrees).
-    var areaToEdge = resolution / 2.0;
+    var halfResolution = resolution / 2.0;
 
     // Use the reference location to pad the supplied short code and decode it.
     var codeArea = decode(
         encode(referenceLatitude, referenceLongitude).substr(0, paddingLength)
         + shortCode);
     // How many degrees latitude is the code from the reference? If it is more
-    // than half the resolution, we need to move it east or west.
-    var degreesDifference = codeArea.latitudeCenter - referenceLatitude;
-    if (degreesDifference > areaToEdge) {
-      // If the center of the short code is more than half a cell east,
-      // then the best match will be one position west.
+    // than half the resolution, we need to move it north or south but keep it
+    // within -90 to 90 degrees.
+    if (referenceLatitude + halfResolution < codeArea.latitudeCenter &&
+        codeArea.latitudeCenter - resolution >= -LATITUDE_MAX_) {
+      // If the proposed code is more than half a cell north of the reference location,
+      // it's too far, and the best match will be one cell south.
       codeArea.latitudeCenter -= resolution;
-    } else if (degreesDifference < -areaToEdge) {
-      // If the center of the short code is more than half a cell west,
-      // then the best match will be one position east.
+    } else if (referenceLatitude - halfResolution > codeArea.latitudeCenter &&
+               codeArea.latitudeCenter + resolution <= LATITUDE_MAX_) {
+      // If the proposed code is more than half a cell south of the reference location,
+      // it's too far, and the best match will be one cell north.
       codeArea.latitudeCenter += resolution;
     }
 
     // How many degrees longitude is the code from the reference?
-    degreesDifference = codeArea.longitudeCenter - referenceLongitude;
-    if (degreesDifference > areaToEdge) {
+    if (referenceLongitude + halfResolution < codeArea.longitudeCenter) {
       codeArea.longitudeCenter -= resolution;
-    } else if (degreesDifference < -areaToEdge) {
+    } else if (referenceLongitude - halfResolution > codeArea.longitudeCenter) {
       codeArea.longitudeCenter += resolution;
     }
 

js/test/test.html

@@ -129,25 +129,20 @@
           // Convert the string numbers to number numbers.
           testFields[1] = parseFloat(testFields[1]);
           testFields[2] = parseFloat(testFields[2]);
-          // Shorten the full length code.
-          var shorten = OpenLocationCode.shorten(
-              testFields[0], testFields[1], testFields[2]);
-          //var shortenBy4 = OpenLocationCode.shortenBy4(
-          //    testFields[0], testFields[1], testFields[2]);
-          //var shortenBy6 = OpenLocationCode.shortenBy6(
-          //    testFields[0], testFields[1], testFields[2]);
-          // Confirm we got what we expected.
-          var coords = ' with ' + testFields[1] + ',' + testFields[2];
-          assert.strictEqual(shorten, testFields[3], testFields[0] + ' shorten ' + coords);
-          //strictEqual(shortenBy4, testFields[3], testFields[0] + ' shortenBy4 ' + coords);
-          //strictEqual(shortenBy6, testFields[4], testFields[0] + ' shortenBy6 ' + coords);
-          // Now try expanding the shortened code.
-          var expanded = OpenLocationCode.recoverNearest(shorten, testFields[1], testFields[2]);
-          assert.strictEqual(expanded, testFields[0], testFields[0] + ' expanding shorten ' + coords);
-          //var expanded = OpenLocationCode.recoverNearest(shortenBy4, testFields[1], testFields[2]);
-          //strictEqual(expanded, testFields[0], testFields[0] + ' expanding shortenBy4 ' + coords);
-          //expanded = OpenLocationCode.recoverNearest(shortenBy6, testFields[1], testFields[2]);
-          //strictEqual(expanded, testFields[0], testFields[0] + ' expanding shortenBy6 ' + coords);
+          var coords = testFields[1] + ',' + testFields[2];
+
+          if (testFields[4] == "B" || testFields[4] == "S") {
+            // Shorten the full length code.
+            var shorten = OpenLocationCode.shorten(
+                testFields[0], testFields[1], testFields[2]);
+            // Confirm we got what we expected.
+            assert.strictEqual(shorten, testFields[3], 'shorten ' + testFields[0] + ' with ' + coords);
+          }
+          if (testFields[4] == "B" || testFields[4] == "R") {
+            // Now try expanding the shortened code.
+            var expanded = OpenLocationCode.recoverNearest(testFields[3], testFields[1], testFields[2]);
+            assert.strictEqual(expanded, testFields[0], 'recover ' + testFields[3] + ' with ' + coords);
+          }
         }
 
         done();

python/openlocationcode.py

@@ -293,25 +293,26 @@ def recoverNearest(shortcode, referenceLatitude, referenceLongitude):
     # The resolution (height and width) of the padded area in degrees.
     resolution = pow(20, 2 - (paddingLength / 2))
     # Distance from the center to an edge (in degrees).
-    areaToEdge = resolution / 2.0
+    halfResolution = resolution / 2.0
     # Use the reference location to pad the supplied short code and decode it.
     codeArea = decode(encode(referenceLatitude, referenceLongitude)[0:paddingLength] + shortcode)
     # How many degrees latitude is the code from the reference? If it is more
-    # than half the resolution, we need to move it east or west.
-    degreesDifference = codeArea.latitudeCenter - referenceLatitude
-    if degreesDifference > areaToEdge:
-        # If the center of the short code is more than half a cell east,
-        # then the best match will be one position west.
+    # than half the resolution, we need to move it north or south but keep it
+    # within -90 to 90 degrees.
+    if (referenceLatitude + halfResolution < codeArea.latitudeCenter and
+        codeArea.latitudeCenter - resolution >= -LATITUDE_MAX_):
+        # If the proposed code is more than half a cell north of the reference location,
+        # it's too far, and the best match will be one cell south.
         codeArea.latitudeCenter -= resolution
-    elif degreesDifference < -areaToEdge:
-        # If the center of the short code is more than half a cell west,
-        # then the best match will be one position east.
+    elif (referenceLatitude - halfResolution > codeArea.latitudeCenter and
+          codeArea.latitudeCenter + resolution <= LATITUDE_MAX_):
+        # If the proposed code is more than half a cell south of the reference location,
+        # it's too far, and the best match will be one cell north.
         codeArea.latitudeCenter += resolution
-    # How many degrees longitude is the code from the reference?
-    degreesDifference = codeArea.longitudeCenter - referenceLongitude
-    if degreesDifference > areaToEdge:
+    # Adjust longitude if necessary.
+    if referenceLongitude + halfResolution < codeArea.longitudeCenter:
         codeArea.longitudeCenter -= resolution
-    elif degreesDifference < -areaToEdge:
+    elif referenceLongitude - halfResolution > codeArea.longitudeCenter:
         codeArea.longitudeCenter += resolution
     return encode(codeArea.latitudeCenter, codeArea.longitudeCenter, codeArea.codeLength)
 

python/openlocationcode_test.py

@@ -36,7 +36,7 @@ def test_shortcodes(self):
 class TestShorten(unittest.TestCase):
     def setUp(self):
         self.testdata = []
-        headermap = {0: 'fullcode', 1: 'lat', 2: 'lng', 3: 'shortcode'}
+        headermap = {0: 'fullcode', 1: 'lat', 2: 'lng', 3: 'shortcode', 4:'testtype'}
         tests_fn = 'test_data/shortCodeTests.csv'
         with open(tests_fn, "r") as fin:
             for line in fin:
@@ -50,7 +50,10 @@ def setUp(self):
 
     def test_full2short(self):
         for td in self.testdata:
-            self.assertEqual(td['shortcode'], olc.shorten(td['fullcode'], td['lat'], td['lng']), td)
+            if td['testtype'] == 'B' or td['testtype'] == 'S':
+                self.assertEqual(td['shortcode'], olc.shorten(td['fullcode'], td['lat'], td['lng']), td)
+            if td['testtype'] == 'B' or td['testtype'] == 'R':
+                self.assertEqual(td['fullcode'], olc.recoverNearest(td['shortcode'], td['lat'], td['lng']), td)
 
 
 class TestEncoding(unittest.TestCase):

ruby/lib/plus_codes/open_location_code.rb

@@ -121,23 +121,21 @@ def recover_nearest(short_code, reference_latitude, reference_longitude)
       code = prefix_by_reference(ref_lat, ref_lng, prefix_len) << short_code
       code_area = decode(code)
 
-      area_range = precision_by_length(prefix_len)
-      area_edge = area_range / 2
+      resolution = precision_by_length(prefix_len)
+      half_res = resolution / 2
 
       latitude = code_area.latitude_center
-      latitude_diff = latitude - ref_lat
-      if (latitude_diff > area_edge)
-        latitude -= area_range
-      elsif (latitude_diff < -area_edge)
-        latitude += area_range
+      if (ref_lat + half_res < latitude && latitude - resolution >= -90)
+        latitude -= resolution
+      elsif (ref_lat - half_res > latitude && latitude + resolution <= 90)
+        latitude += resolution
       end
 
       longitude = code_area.longitude_center
-      longitude_diff = longitude - ref_lng
-      if (longitude_diff > area_edge)
-        longitude -= area_range
-      elsif (longitude_diff < -area_edge)
-        longitude += area_range
+      if (ref_lng + half_res < longitude)
+        longitude -= resolution
+      elsif (ref_lng - half_res > longitude)
+        longitude += resolution
       end
 
       encode(latitude, longitude, code.length - SEPARATOR.length)

ruby/test/plus_codes_test.rb

@@ -47,10 +47,15 @@ def test_shorten
       lat = cols[1].to_f
       lng = cols[2].to_f
       short_code = cols[3]
-      short = @olc.shorten(code, lat, lng)
-      assert_equal(short_code, short)
-      expanded = @olc.recover_nearest(short, lat, lng)
-      assert_equal(code, expanded)
+      test_type = cols[4]
+      if test_type == 'B' || test_type == 'S'
+        short = @olc.shorten(code, lat, lng)
+        assert_equal(short_code, short)
+      end
+      if test_type == 'B' || test_type == 'R'
+        expanded = @olc.recover_nearest(short_code, lat, lng)
+        assert_equal(code, expanded)
+      end
     end
     @olc.shorten('9C3W9QCJ+2VX', 60.3701125, 10.202665625)
   end

test_data/shortCodeTests.csv

@@ -1,18 +1,23 @@
 # Test shortening and extending codes.
 #
 # Format:
-# full code,lat,lng,shortcode
-9C3W9QCJ+2VX,51.3701125,-1.217765625,+2VX
+# full code,lat,lng,shortcode,test_type
+# test_type is R for recovery only, S for shorten only, or B for both.
+9C3W9QCJ+2VX,51.3701125,-1.217765625,+2VX,B
 # Adjust so we can't trim by 8 (+/- .000755)
-9C3W9QCJ+2VX,51.3708675,-1.217765625,CJ+2VX
-9C3W9QCJ+2VX,51.3693575,-1.217765625,CJ+2VX
-9C3W9QCJ+2VX,51.3701125,-1.218520625,CJ+2VX
-9C3W9QCJ+2VX,51.3701125,-1.217010625,CJ+2VX
+9C3W9QCJ+2VX,51.3708675,-1.217765625,CJ+2VX,B
+9C3W9QCJ+2VX,51.3693575,-1.217765625,CJ+2VX,B
+9C3W9QCJ+2VX,51.3701125,-1.218520625,CJ+2VX,B
+9C3W9QCJ+2VX,51.3701125,-1.217010625,CJ+2VX,B
 # Adjust so we can't trim by 6 (+/- .0151)
-9C3W9QCJ+2VX,51.3852125,-1.217765625,9QCJ+2VX
-9C3W9QCJ+2VX,51.3550125,-1.217765625,9QCJ+2VX
-9C3W9QCJ+2VX,51.3701125,-1.232865625,9QCJ+2VX
-9C3W9QCJ+2VX,51.3701125,-1.202665625,9QCJ+2VX
+9C3W9QCJ+2VX,51.3852125,-1.217765625,9QCJ+2VX,B
+9C3W9QCJ+2VX,51.3550125,-1.217765625,9QCJ+2VX,B
+9C3W9QCJ+2VX,51.3701125,-1.232865625,9QCJ+2VX,B
+9C3W9QCJ+2VX,51.3701125,-1.202665625,9QCJ+2VX,B
 # Added to detect error in recoverNearest functionality
-8FJFW222+,42.899,9.012,22+
-796RXG22+,14.95125,-23.5001,22+
+8FJFW222+,42.899,9.012,22+,B
+796RXG22+,14.95125,-23.5001,22+,B
+# Added to detect errors recovering codes near the poles.
+# This tests recovery function, but these codes won't shorten.
+CFX22222+22,89.6,0.0,2222+22,R
+2CXXXXXX+XX,-81.0,0.0,XXXXXX+XX,R