From 6443442aff20c9cb4053178268c6ed93b29d896e Mon Sep 17 00:00:00 2001
From: mutantmonkey <mutantmonkey@mutantmonkey.in>
Date: Sat, 24 Aug 2013 18:45:05 -0700
Subject: [PATCH] improvements to weather module

* use openstreetmap nominatim instead of geonames
* set temperature to None if there is no temp available
* add new test cases to better test geolocation for various places
---
 metar.py                     |  2 ++
 modules/test/test_weather.py | 41 +++++++++++++++++++++++++++++-------
 modules/weather.py           | 36 +++++++++++++++++--------------
 3 files changed, 55 insertions(+), 24 deletions(-)

diff --git a/metar.py b/metar.py
index bc48585..0bbcd12 100644
--- a/metar.py
+++ b/metar.py
@@ -282,6 +282,8 @@ def parse(data):
     if m:
         w.temperature = parse_temp(m.group('temp'))
         w.dewpoint = parse_temp(m.group('dewpoint'))
+    else:
+        w.temperature = None
 
     # pressure
     pressure_re = re.compile(r"([QA])(\d+)")
diff --git a/modules/test/test_weather.py b/modules/test/test_weather.py
index 52a83b2..89c5be8 100644
--- a/modules/test/test_weather.py
+++ b/modules/test/test_weather.py
@@ -13,17 +13,42 @@ class TestWeather(unittest.TestCase):
     def setUp(self):
         self.phenny = MagicMock()
 
-    def test_location(self):
-        name, countryName, lat, lng = location('24060')
+    def test_locations(self):
+        def check_places(*args):
+            def validate(actual_name, actual_lat, actual_lon):
+                names = [n.strip() for n in actual_name.split(',')]
+                for arg in args:
+                    self.assertIn(arg, names)
+            return validate
 
-        self.assertEqual(name, "Blacksburg")
-        self.assertEqual(countryName, "United States")
-        self.assertEqual(lat, 37.2295733)
-        self.assertEqual(lng, -80.4139393)
+        locations = [
+            ('24060', check_places("Blacksburg", "Virginia")),
+            ('92121', check_places("San Diego", "California")),
+            ('94110', check_places("San Francisco", "California")),
+            ('94041', check_places("Mountain View", "California")),
+            ('27959', check_places("Nags Head", "North Carolina")),
+            ('48067', check_places("Royal Oak", "Michigan")),
+            ('23606', check_places("Newport News", "Virginia")),
+            ('23113', check_places("Midlothian", "Virginia")),
+            ('27517', check_places("Chapel Hill", "North Carolina")),
+            ('46530', check_places("Granger", "Indiana")),
+            ('15213', check_places("Pittsburgh", "Pennsylvania")),
+            ('90210', check_places("Beverly Hills", "California")),
+            ('12144', check_places("Clinton Park", "New York")),
+            ('33109', check_places("Homestead", "Florida")),
+            ('80201', check_places("Denver", "Colorado")),
 
-    def test_code(self):
+            ("Berlin", check_places("Berlin", "Deutschland")),
+            ("Paris", check_places("Paris", "European Union")),
+            ("Vilnius", check_places("Vilnius", "European Union")),
+        ]
+
+        for loc, validator in locations:
+            names, lat, lon = location(loc)
+            validator(names, lat, lon)
+
+    def test_code_20164(self):
         icao = code(self.phenny, '20164')
-        
         self.assertEqual(icao, 'KIAD')
 
     def test_airport(self):
diff --git a/modules/weather.py b/modules/weather.py
index 14ec1af..555b64e 100644
--- a/modules/weather.py
+++ b/modules/weather.py
@@ -9,26 +9,27 @@ http://inamidst.com/phenny/
 
 import re
 import metar
+import json
 import web
 from tools import deprecated, GrumbleError
 
 r_from = re.compile(r'(?i)([+-]\d+):00 from')
 
-def location(name):
-    name = web.quote(name)
-    uri = 'http://ws.geonames.org/searchJSON?q=%s&maxRows=1' % name
-    for i in range(10):
-        bytes = web.get(uri)
-        if bytes is not None: break
 
-    results = web.json(bytes)
-    try: name = results['geonames'][0]['name']
-    except IndexError: 
-        return '?', '?', '0', '0'
-    countryName = results['geonames'][0]['countryName']
-    lat = results['geonames'][0]['lat']
-    lng = results['geonames'][0]['lng']
-    return name, countryName, lat, lng
+def location(q):
+    uri = 'http://nominatim.openstreetmap.org/search/?q={query}&format=json'.\
+        format(query=web.quote(q))
+    results = web.get(uri)
+    data = json.loads(results)
+
+    if not 'display_name' in data:
+        return '?', None, None
+
+    display_name = data[0]['display_name']
+    lat = float(data[0]['lat'])
+    lon = float(data[0]['lon'])
+    return display_name, lat, lon
+
 
 def local(icao, hour, minute):
     uri = ('http://www.flightstats.com/' + 
@@ -47,14 +48,16 @@ def local(icao, hour, minute):
         #            ':' + str(minute) + 'Z)')
     return str(hour) + ':' + str(minute) + 'Z'
 
+
 def code(phenny, search):
     from icao import data
     
     if search.upper() in [loc[0] for loc in data]:
         return search.upper()
     else:
-        name, country, latitude, longitude = location(search)
-        if name == '?': return False
+        display_name, latitude, longitude = location(search)
+        if not latitude or not longitude:
+            return False
         sumOfSquares = (99999999999999999999999999999, 'ICAO')
         for icao_code, lat, lon in data:
             latDiff = abs(latitude - lat)
@@ -64,6 +67,7 @@ def code(phenny, search):
                 sumOfSquares = (diff, icao_code)
         return sumOfSquares[1]
 
+
 def f_weather(phenny, input):
     """.weather <ICAO> - Show the weather at airport with the code <ICAO>."""
     icao_code = input.group(2)