sopel-irc / sopel-weather

A working re-implementation of the weather plugin for Sopel
MIT License
6 stars 7 forks source link

Weather erroring on locations that don't have a 'state' #17

Closed Berserkir-Wolf closed 3 years ago

Berserkir-Wolf commented 4 years ago

Running a sopel docker instance, and installed the weather module a while ago. For locations without a 'state' attached, lookup fails with the below:

[2020-08-11 21:30:34,490] sopel.bot            ERROR    - Unexpected error ('state') from dysonparkes at 2020-08-11 21:30:34.489919. Message was: .weather Gisborne, NZ
Traceback (most recent call last):
  File "/home/sopel/.local/lib/python3.8/site-packages/sopel-7.0.6-py3.8.egg/sopel/bot.py", line 606, in call
    exit_code = func(sopel, trigger)
  File "/home/sopel/.sopel/modules/weather/weather.py", line 234, in weather_command
    data = get_weather(bot, trigger)
  File "/home/sopel/.sopel/modules/weather/weather.py", line 197, in get_weather
    latitude, longitude, location = get_geocoords(bot, trigger)
  File "/home/sopel/.sopel/modules/weather/weather.py", line 150, in get_geocoords
    address['state'],
KeyError: 'state'

If I look at the LocationIQ API directly, there's no "state" attribute being passed, and there is definitely a response from LocationIQ for the location being looked up (Gisborne, New Zealand).

[{
    "place_id": "199999",
    "licence": "https://locationiq.com/attribution",
    "osm_type": "node",
    "osm_id": "60269696",
    "boundingbox": ["-38.821326", "-38.501326", "177.8606487", "178.1806487"],
    "lat": "-38.661326",
    "lon": "178.0206487",
    "display_name": "Gisborne, 4010, New Zealand",
    "class": "place",
    "type": "city",
    "importance": 0.7693094441583,
    "icon": "https://locationiq.org/static/images/mapicons/poi_place_city.p.20.png"
}, {
    "place_id": "232435826",
    "licence": "https://locationiq.com/attribution",
    "osm_type": "relation",
    "osm_id": "2643819",
    "boundingbox": ["-38.9749371", "-37.3314453", "177.1221708", "178.8362541"],
    "lat": "-38.2511249",
    "lon": "178.1489099",
    "display_name": "Gisborne, New Zealand",
    "class": "boundary",
    "type": "administrative",
    "importance": 0.73786177459204,
    "icon": "https://locationiq.org/static/images/mapicons/poi_boundary_administrative.p.20.png"
}]

If I query OpenWeatherMap directly using the API key sopel is using, then I get results with a query of "gisborne, nz". Query: http://api.openweathermap.org/data/2.5/weather?q=Gisborne,nz&APPID={APIKEY}&units=metric Result:

{
  "coord": {
    "lon": 178,
    "lat": -38.65
  },
  "weather": [
    {
      "id": 802,
      "main": "Clouds",
      "description": "scattered clouds",
      "icon": "03d"
    }
  ],
  "base": "stations",
  "main": {
    "temp": 10.56,
    "feels_like": 7.83,
    "temp_min": 10.56,
    "temp_max": 10.56,
    "pressure": 1001,
    "humidity": 90
  },
  "visibility": 10000,
  "wind": {
    "speed": 3.58,
    "deg": 236,
    "gust": 7.6
  },
  "clouds": {
    "all": 32
  },
  "dt": 1597181920,
  "sys": {
    "type": 3,
    "id": 2011018,
    "country": "NZ",
    "sunrise": 1597172285,
    "sunset": 1597210093
  },
  "timezone": 43200,
  "id": 2206854,
  "name": "Gisborne",
  "cod": 200
}

None of the responses have a 'state' key, which is as expected. Not all countries use a 'State' system. Line 150 of the module seems to expect/require an entry for state or it fails to resolve. Is there a way to remove this requirement?

dgw commented 4 years ago

Is there a way to remove this requirement?

Sure, use .get() for each of the parts and refactor the string generation to assemble only the relevant pieces.

RustyBower commented 4 years ago

I'll take a look :) I tried testing with some random international locations, but clearly not Gisborne, New Zealand

Berserkir-Wolf commented 4 years ago

NZ is probably a little further out of the way than most places running a sopel instance 🤣 At some point I'll get back into module work. I've not worked on a stock module for a while, got a little spoiled by the spicebot project.

Berserkir-Wolf commented 4 years ago

Looking at the response in Powershell I get the below:

place_id     : 199999
licence      : https://locationiq.com/attribution
osm_type     : node
osm_id       : 60269696
boundingbox  : {-38.821326, -38.501326, 177.8606487, 178.1806487}
lat          : -38.661326
lon          : 178.0206487
display_name : Gisborne, 4010, New Zealand
class        : place
type         : city
importance   : 0.7593094441583
icon         : https://locationiq.org/static/images/mapicons/poi_place_city.p.20.png
address      : @{city=Gisborne; region=Gisborne; postcode=4010; country=New Zealand; country_code=nz}

Lat, Long, and Address are all there - but no state or county. As a comparison to a US location: Gisborne: address : @{city=Gisborne; region=Gisborne; postcode=4010; country=New Zealand; country_code=nz} Boston: address : @{city=Boston; county=Suffolk County; state=Massachusetts; postcode=02203; country=United States of America; country_code=us}

Maybe a clause for 'if $r.keys contains city and not $r.keys contains state then format = city, region, country' or similar?

Berserkir-Wolf commented 3 years ago

Right, after some further testing... Replacing

    if 'city' in address.keys():

with

    if 'region' in address.keys():
        location = '{}, {}, {}'.format(address['city'],
                                       address['region'],
                                       address['country_code'].upper())
    elif 'city' in address.keys():

allows querying directly (.weather Gisborne, NZ) but fails to setlocation with Unexpected error (Unable to geocode) from dysonparkes at 2020-12-10 21:55:57.066577. Message was: .weather setlocation Gisborne, NZ

I am able to get weather by just giving a single city (Gisborne), but cannot set the location. It's a little strange.