Repository: geopy/geopy (python)

Issue:
Point normalizes latitude bizarrely
Point treats latitude periodically and thus connects the north and south
poles

~~~
>>> from geopy import Point
>>> Point(90.1, 10)
Point(-89.9, 10.0, 0.0)
~~~

This can't be considered a sensible result.  Better would be to change
latitudes outside the range [-90, 90] into nans.  Then people who
accidentally switch the order of latitude and longitude will sometimes
get a sharp poke in the eye that they've done something wrong.


Pull request:
Raise ValueError instead of latitude normalization
Closes #243

Patch:
diff --git a/geopy/geocoders/geonames.py b/geopy/geocoders/geonames.py
index 1e8c41b..64f3818 100644
--- a/geopy/geocoders/geonames.py
+++ b/geopy/geocoders/geonames.py
@@ -1,6 +1,7 @@
 """
 :class:`GeoNames` geocoder.
 """
+import warnings
 
 from geopy.compat import urlencode
 from geopy.exc import (
@@ -107,7 +108,7 @@ class GeoNames(Geocoder): # pylint: disable=W0223
     def reverse(
             self,
             query,
-            exactly_one=False,
+            exactly_one=DEFAULT_SENTINEL,
             timeout=DEFAULT_SENTINEL,
     ):
         """
@@ -123,12 +124,26 @@ class GeoNames(Geocoder): # pylint: disable=W0223
         :param bool exactly_one: Return one result or a list of results, if
             available.
 
+            .. versionchanged:: 1.14.0
+               Default value for ``exactly_one`` was ``False``, which differs
+               from the conventional default across geopy. Please always pass
+               this argument explicitly, otherwise you would get a warning.
+               In geopy 2.0 the default value will become ``True``.
+
         :param int timeout: Time, in seconds, to wait for the geocoding service
             to respond before raising a :class:`geopy.exc.GeocoderTimedOut`
             exception. Set this only if you wish to override, on this call
             only, the value set during the geocoder's initialization.
 
         """
+        if exactly_one is DEFAULT_SENTINEL:
+            warnings.warn('%s.reverse: default value for `exactly_one` '
+                          'argument will become True in geopy 2.0. '
+                          'Specify `exactly_one=False` as the argument '
+                          'explicitly to get rid of this warning.' % type(self).__name__,
+                          DeprecationWarning)
+            exactly_one = False
+
         try:
             lat, lng = [
                 x.strip() for x in
diff --git a/geopy/geocoders/googlev3.py b/geopy/geocoders/googlev3.py
index e7f01c4..0370e45 100644
--- a/geopy/geocoders/googlev3.py
+++ b/geopy/geocoders/googlev3.py
@@ -5,6 +5,7 @@
 import base64
 import hashlib
 import hmac
+import warnings
 
 from geopy.compat import urlencode
 from geopy.exc import (
@@ -159,7 +160,7 @@ class GoogleV3(Geocoder):  # pylint: disable=R0902
 
     def geocode(
             self,
-            query,
+            query=None,
             exactly_one=True,
             timeout=DEFAULT_SENTINEL,
             bounds=None,
@@ -171,7 +172,14 @@ class GoogleV3(Geocoder):  # pylint: disable=R0902
         """
         Geocode a location query.
 
-        :param str query: The address or query you wish to geocode.
+        :param str query: The address or query you wish to geocode. Optional,
+            if ``components`` param is set::
+
+                >>> g = GoogleV3()
+                >>> g.geocode(components={"city": "Paris", "country": "FR"})
+
+            .. versionchanged:: 1.14.0
+               Now ``query`` is optional if ``components`` param is set.
 
         :param bool exactly_one: Return one result or a list of results, if
             available.
@@ -197,9 +205,12 @@ class GoogleV3(Geocoder):  # pylint: disable=R0902
             device with a location sensor.
         """
         params = {
-            'address': self.format_string % query,
             'sensor': str(sensor).lower()
         }
+        if query is None and not components:
+            raise ValueError('Either `query` or `components` must be set.`')
+        if query is not None:
+            params['address'] = self.format_string % query
         if self.api_key:
             params['key'] = self.api_key
         if bounds:
@@ -228,7 +239,7 @@ class GoogleV3(Geocoder):  # pylint: disable=R0902
     def reverse(
             self,
             query,
-            exactly_one=False,
+            exactly_one=DEFAULT_SENTINEL,
             timeout=DEFAULT_SENTINEL,
             language=None,
             sensor=False,
@@ -244,6 +255,12 @@ class GoogleV3(Geocoder):  # pylint: disable=R0902
         :param bool exactly_one: Return one result or a list of results, if
             available.
 
+            .. versionchanged:: 1.14.0
+               Default value for ``exactly_one`` was ``False``, which differs
+               from the conventional default across geopy. Please always pass
+               this argument explicitly, otherwise you would get a warning.
+               In geopy 2.0 the default value will become ``True``.
+
         :param int timeout: Time, in seconds, to wait for the geocoding service
             to respond before raising a :class:`geopy.exc.GeocoderTimedOut`
             exception. Set this only if you wish to override, on this call
@@ -254,6 +271,14 @@ class GoogleV3(Geocoder):  # pylint: disable=R0902
         :param bool sensor: Whether the geocoding request comes from a
             device with a location sensor.
         """
+        if exactly_one is DEFAULT_SENTINEL:
+            warnings.warn('%s.reverse: default value for `exactly_one` '
+                          'argument will become True in geopy 2.0. '
+                          'Specify `exactly_one=False` as the argument '
+                          'explicitly to get rid of this warning.' % type(self).__name__,
+                          DeprecationWarning)
+            exactly_one = False
+
         params = {
             'latlng': self._coerce_point_to_string(query),
             'sensor': str(sensor).lower()
diff --git a/geopy/geocoders/ignfrance.py b/geopy/geocoders/ignfrance.py
index d6bd2c7..6944768 100644
--- a/geopy/geocoders/ignfrance.py
+++ b/geopy/geocoders/ignfrance.py
@@ -1,7 +1,7 @@
 """
 :class:`.IGNFrance` is the IGN France Geocoder.
 """
-
+import warnings
 import xml.etree.ElementTree as ET
 
 from geopy.compat import (HTTPBasicAuthHandler, HTTPPasswordMgrWithDefaultRealm, Request,
@@ -238,7 +238,7 @@ class IGNFrance(Geocoder):   # pylint: disable=W0223
             reverse_geocode_preference=('StreetAddress', ),
             maximum_responses=25,
             filtering='',
-            exactly_one=False,
+            exactly_one=DEFAULT_SENTINEL,
             timeout=DEFAULT_SENTINEL,
     ):
         """
@@ -263,12 +263,25 @@ class IGNFrance(Geocoder):   # pylint: disable=W0223
         :param bool exactly_one: Return one result or a list of results, if
             available.
 
+            .. versionchanged:: 1.14.0
+               Default value for ``exactly_one`` was ``False``, which differs
+               from the conventional default across geopy. Please always pass
+               this argument explicitly, otherwise you would get a warning.
+               In geopy 2.0 the default value will become ``True``.
+
         :param int timeout: Time, in seconds, to wait for the geocoding service
             to respond before raising a :class:`geopy.exc.GeocoderTimedOut`
             exception. Set this only if you wish to override, on this call
             only, the value set during the geocoder's initialization.
 
         """
+        if exactly_one is DEFAULT_SENTINEL:
+            warnings.warn('%s.reverse: default value for `exactly_one` '
+                          'argument will become True in geopy 2.0. '
+                          'Specify `exactly_one=False` as the argument '
+                          'explicitly to get rid of this warning.' % type(self).__name__,
+                          DeprecationWarning)
+            exactly_one = False
 
         sub_request = """
             <ReverseGeocodeRequest>
diff --git a/geopy/geocoders/opencage.py b/geopy/geocoders/opencage.py
index f5ad0ea..77af533 100644
--- a/geopy/geocoders/opencage.py
+++ b/geopy/geocoders/opencage.py
@@ -1,6 +1,7 @@
 """
 :class:`.OpenCage` is the Opencagedata geocoder.
 """
+import warnings
 
 from geopy.compat import urlencode
 from geopy.exc import GeocoderQueryError, GeocoderQuotaExceeded
@@ -137,7 +138,7 @@ class OpenCage(Geocoder):
             self,
             query,
             language=None,
-            exactly_one=False,
+            exactly_one=DEFAULT_SENTINEL,
             timeout=DEFAULT_SENTINEL,
     ):
         """
@@ -153,12 +154,26 @@ class OpenCage(Geocoder):
         :param bool exactly_one: Return one result or a list of results, if
             available.
 
+            .. versionchanged:: 1.14.0
+               Default value for ``exactly_one`` was ``False``, which differs
+               from the conventional default across geopy. Please always pass
+               this argument explicitly, otherwise you would get a warning.
+               In geopy 2.0 the default value will become ``True``.
+
         :param int timeout: Time, in seconds, to wait for the geocoding service
             to respond before raising a :class:`geopy.exc.GeocoderTimedOut`
             exception. Set this only if you wish to override, on this call
             only, the value set during the geocoder's initialization.
 
         """
+        if exactly_one is DEFAULT_SENTINEL:
+            warnings.warn('%s.reverse: default value for `exactly_one` '
+                          'argument will become True in geopy 2.0. '
+                          'Specify `exactly_one=False` as the argument '
+                          'explicitly to get rid of this warning.' % type(self).__name__,
+                          DeprecationWarning)
+            exactly_one = False
+
         params = {
             'key': self.api_key,
             'q': self._coerce_point_to_string(query),
diff --git a/geopy/geocoders/yandex.py b/geopy/geocoders/yandex.py
index eafa609..eab68d4 100644
--- a/geopy/geocoders/yandex.py
+++ b/geopy/geocoders/yandex.py
@@ -1,6 +1,7 @@
 """
 :class:`Yandex` geocoder.
 """
+import warnings
 
 from geopy.compat import urlencode
 from geopy.exc import GeocoderParseError, GeocoderServiceError
@@ -107,7 +108,7 @@ class Yandex(Geocoder): # pylint: disable=W0223
     def reverse(
             self,
             query,
-            exactly_one=False,
+            exactly_one=DEFAULT_SENTINEL,
             timeout=DEFAULT_SENTINEL,
             kind=None,
     ):
@@ -122,6 +123,12 @@ class Yandex(Geocoder): # pylint: disable=W0223
         :param bool exactly_one: Return one result or a list of results, if
             available.
 
+            .. versionchanged:: 1.14.0
+               Default value for ``exactly_one`` was ``False``, which differs
+               from the conventional default across geopy. Please always pass
+               this argument explicitly, otherwise you would get a warning.
+               In geopy 2.0 the default value will become ``True``.
+
         :param int timeout: Time, in seconds, to wait for the geocoding service
             to respond before raising a :class:`geopy.exc.GeocoderTimedOut`
             exception. Set this only if you wish to override, on this call
@@ -132,6 +139,14 @@ class Yandex(Geocoder): # pylint: disable=W0223
 
             .. versionadded:: 1.14.0
         """
+        if exactly_one is DEFAULT_SENTINEL:
+            warnings.warn('%s.reverse: default value for `exactly_one` '
+                          'argument will become True in geopy 2.0. '
+                          'Specify `exactly_one=False` as the argument '
+                          'explicitly to get rid of this warning.' % type(self).__name__,
+                          DeprecationWarning)
+            exactly_one = False
+
         try:
             lat, lng = [
                 x.strip() for x in
diff --git a/geopy/point.py b/geopy/point.py
index ab32add..94b1b82 100644
--- a/geopy/point.py
+++ b/geopy/point.py
@@ -68,22 +68,18 @@ def _normalize_coordinates(latitude, longitude, altitude):
 
     is_all_finite = all(isfinite(x) for x in (latitude, longitude, altitude))
     if not is_all_finite:
-        warnings.warn('Point coordinates should be finite. NaN or inf has '
-                      'been passed as a coordinate.  This will cause a '
-                      'ValueError exception in the future versions of geopy.',
-                      UserWarning)
+        raise ValueError('Point coordinates must be finite. %r has been passed '
+                         'as coordinates.' % ((latitude, longitude, altitude),))
 
     if abs(latitude) > 90:
-        warnings.warn('Latitude has been normalized, because its '
-                      'absolute value was more than 90. This is probably '
-                      'not what was meant, because the normalized value '
-                      'is on a different pole. If you pass coordinates as '
-                      'positional args, please make sure that the order is '
-                      '(latitude, longitude) or (y, x) in Cartesian terms.  '
-                      'This will cause a ValueError exception in the future '
-                      'versions of geopy.',
+        warnings.warn('Latitude normalization has been prohibited in the newer '
+                      'versions of geopy, because the normalized value happened '
+                      'to be on a different pole, which is probably not what was '
+                      'meant. If you pass coordinates as positional args, '
+                      'please make sure that the order is '
+                      '(latitude, longitude) or (y, x) in Cartesian terms.',
                       UserWarning)
-        latitude = _normalize_angle(latitude, 90.0)
+        raise ValueError('Latitude must be in the [-90; 90] range.')
 
     if abs(longitude) > 180:
         # Longitude normalization is pretty straightforward and doesn't seem


Test patch:
diff --git a/test/geocoders/geonames.py b/test/geocoders/geonames.py
index f2feacd..22af7b8 100644
--- a/test/geocoders/geonames.py
+++ b/test/geocoders/geonames.py
@@ -44,6 +44,6 @@ class GeoNamesTestCase(GeocoderTestBase):
         # work around ConfigurationError raised in GeoNames init
         self.geocoder = GeoNames(username=env['GEONAMES_USERNAME'])
         self.reverse_run(
-            {"query": "40.75376406311989, -73.98489005863667"},
+            {"query": "40.75376406311989, -73.98489005863667", "exactly_one": True},
             {"latitude": 40.75376406311989, "longitude": -73.98489005863667},
         )
diff --git a/test/geocoders/googlev3.py b/test/geocoders/googlev3.py
index 1f24b56..f161b72 100644
--- a/test/geocoders/googlev3.py
+++ b/test/geocoders/googlev3.py
@@ -131,7 +131,7 @@ class GoogleV3TestCase(GeocoderTestBase): # pylint: disable=R0904,C0111
             {"latitude": 39.916, "longitude": 116.390},
         )
 
-    def test_geocode_components(self):
+    def test_geocode_with_conflicting_components(self):
         """
         GoogleV3.geocode returns None on conflicting components
         """
@@ -161,12 +161,20 @@ class GoogleV3TestCase(GeocoderTestBase): # pylint: disable=R0904,C0111
             {"latitude": 28.4636296, "longitude": -16.2518467},
         )
 
+    def test_components_without_query(self):
+        self.geocode_run(
+            {
+                "components": {"city": "Paris", "country": "FR"},
+            },
+            {"latitude": 46.227638, "longitude": 2.213749},
+        )
+
     def test_reverse_string(self):
         """
         GoogleV3.reverse string
         """
         self.reverse_run(
-            {"query": "40.75376406311989, -73.98489005863667"},
+            {"query": "40.75376406311989, -73.98489005863667", "exactly_one": True},
             {"latitude": 40.75376406311989, "longitude": -73.98489005863667},
         )
 
@@ -175,7 +183,7 @@ class GoogleV3TestCase(GeocoderTestBase): # pylint: disable=R0904,C0111
         GoogleV3.reverse Point
         """
         self.reverse_run(
-            {"query": self.new_york_point},
+            {"query": self.new_york_point, "exactly_one": True},
             {"latitude": 40.75376406311989, "longitude": -73.98489005863667},
         )
 
diff --git a/test/geocoders/ignfrance.py b/test/geocoders/ignfrance.py
index 003f47e..077e199 100644
--- a/test/geocoders/ignfrance.py
+++ b/test/geocoders/ignfrance.py
@@ -253,6 +253,7 @@ class IGNFranceTestCase(GeocoderTestBase):
         res = self._make_request(
             self.geocoder.reverse,
             query='47.229554,-1.541519',
+            exactly_one=False,
             reverse_geocode_preference=['StreetAddress', 'PositionOfInterest']
         )
         self.assertEqual(
@@ -278,12 +279,14 @@ class IGNFranceTestCase(GeocoderTestBase):
         res_call_radius = self._make_request(
             self.geocoder.reverse,
             query='48.8033333,2.3241667',
+            exactly_one=False,
             maximum_responses=10,
             filtering=spatial_filtering_radius)
 
         res_call = self._make_request(
             self.geocoder.reverse,
             query='48.8033333,2.3241667',
+            exactly_one=False,
             maximum_responses=10
         )
 
diff --git a/test/geocoders/util.py b/test/geocoders/util.py
index 66a3531..81a1f89 100644
--- a/test/geocoders/util.py
+++ b/test/geocoders/util.py
@@ -56,7 +56,8 @@ class GeocoderTestBase(unittest.TestCase): # pylint: disable=R0904
                 self.fail('No result found')
             else:
                 return
-        self._verify_request(result, **expected)
+        self._verify_request(result, exactly_one=payload.get('exactly_one', True),
+                             **expected)
         return result
 
     def reverse_run(self, payload, expected, expect_failure=False):
@@ -69,7 +70,8 @@ class GeocoderTestBase(unittest.TestCase): # pylint: disable=R0904
                 self.fail('No result found')
             else:
                 return
-        self._verify_request(result, **expected)
+        self._verify_request(result, exactly_one=payload.get('exactly_one', True),
+                             **expected)
         return result
 
     @staticmethod
@@ -94,11 +96,12 @@ class GeocoderTestBase(unittest.TestCase): # pylint: disable=R0904
             latitude=EMPTY,
             longitude=EMPTY,
             address=EMPTY,
-        ):
+            exactly_one=True,
+    ):
         """
         Verifies that a a result matches the kwargs given.
         """
-        item = result[0] if isinstance(result, (tuple, list)) else result
+        item = result if exactly_one else result[0]
 
         if raw is not EMPTY:
             self.assertEqual(item.raw, raw)
diff --git a/test/geocoders/yandex.py b/test/geocoders/yandex.py
index e0365de..5764fd3 100644
--- a/test/geocoders/yandex.py
+++ b/test/geocoders/yandex.py
@@ -64,7 +64,7 @@ class YandexTestCase(GeocoderTestBase):
         """
         self.geocoder = Yandex()
         self.reverse_run(
-            {"query": "40.75376406311989, -73.98489005863667"},
+            {"query": "40.75376406311989, -73.98489005863667", "exactly_one": True},
             {"latitude": 40.75376406311989, "longitude": -73.98489005863667},
         )
 
diff --git a/test/test_distance.py b/test/test_distance.py
index 22035e2..339c310 100644
--- a/test/test_distance.py
+++ b/test/test_distance.py
@@ -78,19 +78,14 @@ class CommonDistanceComputationCases(object):
         self.assertAlmostEqual(point.latitude, 0.0)
         self.assertAlmostEqual(point.longitude, 179.91, 3)
 
-    def test_should_tolerate_nans(self):
+    def test_should_not_tolerate_nans(self):
         nan = float('nan')
-        # This is probably a bad design to silently turn NaNs into NaNs
-        # instead of raising a ValueError, but this is the current behaviour,
-        # hence this test.
-        with warnings.catch_warnings(record=True) as w:
-            warnings.simplefilter('always')
-            self.assertTrue(math.isnan(self.cls((nan, nan), (1, 1)).kilometers))
-            self.assertEqual(1, len(w))
-            self.assertTrue(math.isnan(self.cls((nan, 1), (1, nan)).kilometers))
-            self.assertEqual(3, len(w))  # 1 per each point tuple
-            self.assertTrue(math.isnan(self.cls((nan, 1), (nan, 1)).kilometers))
-            self.assertEqual(5, len(w))
+        with self.assertRaises(ValueError):
+            self.cls((nan, nan), (1, 1)).kilometers
+        with self.assertRaises(ValueError):
+            self.cls((nan, 1), (1, nan)).kilometers
+        with self.assertRaises(ValueError):
+            self.cls((nan, 1), (nan, 1)).kilometers
 
     def test_should_compute_distance_for_multiple_points_pairwise(self):
         dist_total = self.cls((10, 20), (40, 60), (0, 80), (0, 10))
@@ -125,8 +120,9 @@ class CommonDistanceComputationCases(object):
         with warnings.catch_warnings(record=True) as w:
             warnings.simplefilter('always')
             # The correct order is (lat, lon).
-            self.cls((lon, lat), (lon - 10, lat))
-            self.assertEqual(2, len(w))  # 1 per each point tuple
+            with self.assertRaises(ValueError):
+                self.cls((lon, lat), (lon - 10, lat))
+            self.assertEqual(1, len(w))
 
 
 class CommonMathematicalOperatorCases(object):
diff --git a/test/test_point.py b/test/test_point.py
index 0b5005c..34e364f 100644
--- a/test/test_point.py
+++ b/test/test_point.py
@@ -124,14 +124,22 @@ class PointTestCase(unittest.TestCase):
     def test_point_degrees_are_normalized(self):
         with warnings.catch_warnings(record=True) as w:
             warnings.simplefilter('always')
-            point = Point(95, 185, 375)
-            self.assertEqual((-85, -175, 375), tuple(point))
+            with self.assertRaises(ValueError):
+                # Latitude normalization is not allowed
+                Point(95, 185, 375)
             self.assertEqual(1, len(w))
 
-            point = Point(-95, -185, 375)
-            self.assertEqual((85, 175, 375), tuple(point))
+            with self.assertRaises(ValueError):
+                # Latitude normalization is not allowed
+                Point(-95, -185, 375)
             self.assertEqual(2, len(w))
 
+            point = Point(-85, 185, 375)
+            self.assertEqual((-85, -175, 375), tuple(point))
+
+            point = Point(85, -185, 375)
+            self.assertEqual((85, 175, 375), tuple(point))
+
             point = Point(-0.0, -0.0, 375)
             self.assertEqual((0.0, 0.0, 375.0), tuple(point))  # note that the zeros might be negative
             # ensure that negative zeros are normalized to the positive ones
@@ -142,13 +150,12 @@ class PointTestCase(unittest.TestCase):
             self.assertEqual((-90, -180, 375), tuple(point))
             self.assertEqual(2, len(w))
 
-            point = Point(-270, -540, 375)
+            point = Point(-90, -540, 375)
             self.assertEqual((-90, -180, 375), tuple(point))
-            self.assertEqual(3, len(w))
 
-            point = Point(270, 540, 375)
-            self.assertEqual((-90, -180, 375), tuple(point))
-            self.assertEqual(4, len(w))
+            point = Point(90, 540, 375)
+            self.assertEqual((90, -180, 375), tuple(point))
+            self.assertEqual(2, len(w))
 
     def test_point_degrees_normalization_does_not_lose_precision(self):
         if sys.float_info.mant_dig != 53:
@@ -173,13 +180,10 @@ class PointTestCase(unittest.TestCase):
         # (180.00000000000003 + 180) fraction doesn't fit in 52 bits.
         #
         # This test ensures that such unwanted precision loss is not happening.
-        with warnings.catch_warnings(record=True) as w:
-            warnings.simplefilter('always')
-            self.assertEqual(tuple(Point(90.00000000000002, 180.00000000000003)),
-                             (-89.99999999999998, -179.99999999999997, 0))
-            self.assertEqual(tuple(Point(9.000000000000002, 1.8000000000000003)),
-                             (9.000000000000002, 1.8000000000000003, 0))
-            self.assertEqual(1, len(w))
+        self.assertEqual(tuple(Point(-89.99999999999998, 180.00000000000003)),
+                         (-89.99999999999998, -179.99999999999997, 0))
+        self.assertEqual(tuple(Point(9.000000000000002, 1.8000000000000003)),
+                         (9.000000000000002, 1.8000000000000003, 0))
 
     def test_unpacking(self):
         point = Point(self.lat, self.lon, self.alt)
@@ -241,14 +245,19 @@ class PointTestCase(unittest.TestCase):
         point = Point()
         with warnings.catch_warnings(record=True) as w:
             warnings.simplefilter('always')
-            point[0] = 100
+            with self.assertRaises(ValueError):
+                point[0] = 100
+            self.assertEqual(1, len(w))
+            self.assertEqual((0, 0, 0), tuple(point))
+            point[0] = -80
             point[1] = 200
             # Please note that attribute assignments are not normalized.
             # Only __setitem__ assignments are.
             self.assertEqual((-80, -160, 0), tuple(point))
-            self.assertEqual(1, len(w))  # for latitude
-            point[1] = float("nan")
-            self.assertEqual(2, len(w))
+            with self.assertRaises(ValueError):
+                point[1] = float("nan")
+            self.assertEqual(1, len(w))
+            self.assertEqual((-80, -160, 0), tuple(point))
 
     def test_point_assign_coordinates(self):
         point = Point(self.lat + 10, self.lon + 10, self.alt + 10)