csparpa / pyowm

A Python wrapper around the OpenWeatherMap web API
https://pyowm.readthedocs.io
MIT License
791 stars 175 forks source link

owm.city_id_registry() incompatible on Python 2.7.14 #219

Closed RaenonX closed 6 years ago

RaenonX commented 6 years ago

I ran the code below:

import pyowm, traceback

OWM_KEY = 'xxxxx'

if __name__ == "__main__":
    owm_client = pyowm.OWM(OWM_KEY)
    city_id_reg = owm_client.city_id_registry()

    try:
        city_id_reg.ids_for('Tokyo')
    except:
        traceback.print_exc()

And got the these things:

Traceback (most recent call last):
  File "C:\Users\RaenonX\OneDrive\SkyDrive\Programming Projects\LineBotLocalTest\app.py", line 12, in <module>
    city_id_reg.ids_for('Tokyo')
  File "C:\Python27-x64\lib\site-packages\pyowm\webapi25\cityidregistry.py", line 76, in ids_for
    splits = self._filter_matching_lines(city_name, country, matching)
  File "C:\Python27-x64\lib\site-packages\pyowm\webapi25\cityidregistry.py", line 151, in _filter_matching_lines
    lines = [l.strip() for l in self._get_lines(filename)]
  File "C:\Python27-x64\lib\site-packages\pyowm\webapi25\cityidregistry.py", line 198, in _get_lines
    with gzip.open(res_name, "rt") as fh:
  File "C:\Python27-x64\lib\gzip.py", line 34, in open
    return GzipFile(filename, mode, compresslevel)
  File "C:\Python27-x64\lib\gzip.py", line 94, in __init__
    fileobj = self.myfileobj = __builtin__.open(filename, mode or 'rb')
ValueError: Invalid mode ('rtb')

Does anyone has the same problem as mine?

And I found this forum that says this is not the bug, should use the code like:

if sys.version_info >= (3,):
    handle = gzip.open(filename, "rt")
else:
    handle = gzip.open(filename)

to fix it.

I hope that copying whole package and just edit one line will not be necessary :rofl:

RaenonX commented 6 years ago

Additional Information:

I forgot to mention that my Python is running on Windows :rofl:

And I did some "research" on the source code and extracted the CityIDRegistry class. After I replace: with gzip.open(res_name, "rt") as fh: (Line 198) with with gzip.open(res_name, "r") as fh:

IT WORKS 😄

Code:

# -*- coding: utf-8 -*-

import tool

if __name__ == "__main__":
    reg = tool.weather.cityids.CityIDRegistry('%03d-%03d.txt.gz')
    print reg.ids_for('taichung')

Output (Console): [(1668399, u'Taichung', u'TW')]

csparpa commented 6 years ago

Hi @RaenonX I've run your very same code

and got no errors, everything was smooth

That said, we have tests covering CityIDRegistry class: they would fail if there was a bug, but they don't

Anyway, the read mode rt seems to be redundant (the default read mode is always t in Python), I will fix it

Thanks for reporting

RaenonX commented 6 years ago

Probably it's because the platform In using? I got that when I ran the code on Windows

On Dec 1, 2017 4:14 AM, "Claudio Sparpaglione" notifications@github.com wrote:

Hi @RaenonX https://github.com/raenonx I've run your very same code

  • on a container from the official Docker Python 2.7.14 image
  • on my Linux box, which also has Python 2.7.14 installed

and got no errors, everything was smooth

That said, we have tests https://github.com/csparpa/pyowm/blob/develop/tests/integration/webapi25/test_cityidregistry_reads_fs.py covering CityIDRegistry class: they would fail if there was a bug, but they don't

Anyway, the read mode rt seems to be redundant (the default read mode is always t in Python), I will fix it

Thanks for reporting

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/csparpa/pyowm/issues/219#issuecomment-348307745, or mute the thread https://github.com/notifications/unsubscribe-auth/ARlt3xJm43xdl7eg2lR7mCTV3CsO67gkks5s7wyUgaJpZM4Qu9T8 .

csparpa commented 6 years ago

Hi @RaenonX I've finally found time to test the potential bug on Windows and you are right: the issue happens on Python 2.7.x

I've also tested your fix and it works nice :)

So thanks again for reporting!

csparpa commented 6 years ago

Fixed by https://github.com/csparpa/pyowm/commit/43ff6128f3d84d5ce9b1ad9e0e2b6efcda46c499