search

panodata / dwdweather2

Python client to access weather data from Deutscher Wetterdienst (DWD), the federal meteorological service in Germany.
https://community.panodata.org/t/dwdweather2-a-python-client-to-access-weather-data-from-deutscher-wetterdienst-dwd/98
MIT License
72 stars 13 forks source link

Support for Python3 #7

Closed amotl closed 4 years ago

amotl commented 5 years ago

@Noordsestern asked over at #5

Any plans migrating to python3?

amotl commented 5 years ago

We recognized there already have been numerous contributions to bring the original dwd-weather project to the world of Python3, like

So, after the migration and refactoring has settled a bit and produced a reasonably stable program right now, we would love to get respective contributions to finally be able to run this program on Python 3.

The original contributions will not be helpful here as the refactoring went pretty far including modularizing the code, switching from FTP to HTTP and some other things we already have been working on. We are sorry for that mismatch and that we haven't been able to cherry-pick the respective improvements beforehand.

Bottom line

So,

Any plans migrating to python3?

Contributions to that are very much appreciated.

With kind regards, Andreas.

amotl commented 5 years ago

Dear @Noordsestern,

we can see you are already working on [1] - thank's for that! We just wanted to let you know you should probably always add the --reset-cache option when developing in order to always touch all available code paths. Otherwise, the acquisition machinery will get passed-by as the data is solely coming from the SQLite database.

Good luck with further Python3-porting, we appreciate your work on that and will be happy about the outcome.

With kind regards, Andreas.

[1] https://github.com/Noordsestern/dwdweather2

jeremiahpslewis commented 5 years ago

What versions of Python does the library currently target? There are a number of code optimizations (some already laid out in the comments) which could be introduced if the library is 3.X+. Given that 2.7 is nearly totally deprecated and this is library is still in beta, it seems reasonable to move to Python 3 only, which leads to the question, can we limit support to 3.7+?

amotl commented 5 years ago

Dear Jeremiah,

supporting 3.7+ is totally reasonable and dropping support for 2.7 is long overdue. Please go ahead, we appreciate your work on that very much.

With kind regards, Andreas.

amotl commented 5 years ago

Hi there,

we just revisited #8 and found that @wtfuii already added appropriate amendments:

with my changes, the lib should be compatible with Python 3.6

So, I am just wondering whether we might have overlooked this here?

With kind regards, Andreas.

Noordsestern commented 5 years ago

Can't tell. But I'd say go with it and if anything incompatible comes up, we fix it :) unittests might be good in any case, though.

amotl commented 5 years ago

Hi there,

while working on #17, we have been running dwdweather2 on Python 3.7.4 and it worked well so far. However, we just invoked a single command [1]. Please let us know about your experiences.

With kind regards, Andreas.

[1] dwdweather weather 5856 20190717T11 --resolution hourly --categories solar

mmaelicke commented 4 years ago

Hey there, Maybe related here. I just downloaded and run in a Python 3.7.6 and it did not work on a Windows machine. I don't have my Linux os available right now, to test there, but here is the traceback anyway:

image

At first glance, this seems to be solvable. I will have a look into it anyway (as soon as there is time, bit busy right now), but just wanted to let you know.

amotl commented 4 years ago

Dear @mmaelicke,

thanks for writing in. I've just tried it again on CPython 3.7.4 and CPython 3.8.0 on macOS and importing ParserError from dateutil.parser works perfectly.

$ python
Python 3.8.0 (default, Dec 12 2019, 00:10:11)
[Clang 10.0.0 (clang-1000.11.45.5)] on darwin
Type "help", "copyright", "credits" or "license" for more information.

>>> from dateutil.parser import parse, ParserError

It makes me sad that it would not work for you out of the box on Windows for some reason. Maybe this is related to Anaconda in any way? Can I humbly ask you to share this output with us?

>>> import dateutil
>>> dateutil.__version__
'2.8.1'
>>> dateutil.__path__
['/Users/amo/dev/tmp/python-testdrive/.venv3/lib/python3.8/site-packages/dateutil']

With kind regards, Andreas.

amotl commented 4 years ago

Dear Mirko,

Maybe this is related to Anaconda in any way?

[1] tells us that ParserError has been introduced through [2] with python-dateutil==2.8.1. However, [3] tells us that the Anaconda binary release package for all architectures including win-64 still uses python-dateutil==2.8.0. On the other hand, the noarch release package is already on 2.8.1.

Can I ask you how you actually installed dwdweather2 within your Anaconda environment? Do you see any way of installing python-dateutil==2.8.1 - either through the noarch variant or by using pip?

With kind regards, Andreas.

[1] https://build.opensuse.org/package/view_file/openSUSE:Factory/python-python-dateutil/python-python-dateutil.changes [2] https://github.com/dateutil/dateutil/pull/881 [3] https://anaconda.org/anaconda/python-dateutil

amotl commented 4 years ago

Hi again,

8df0f3dd relaxes things a bit, I've just pushed dwdweather2-0.12.1 which might help you going forward on Anaconda without any hassle.

With kind regards, Andreas.

mmaelicke commented 4 years ago

Hey Andreas, Thanks for the quick reply. I did just quickly run pip install dwdweather2 and, honestly, did not look any further into the issue, as I was a bit of in a hurry.

Back on my actual working machine, using Linux, there are no issues in installing dwdweather2 and downloading some random data (following examples in the readme). Can confirm for Py36 and Py37, here.

On the windows machine, I have a stupid dependency on dateutil <= 2.8.0 and I am not exactly sure why. Howerver, installing dwdweather2 into a clean anaconda env worked fine with python-dateutil>=2.8.1. So I guess everthing is fine. Thanks a lot! The only thing that could be considered is to update the requirements.txt

mmaelicke commented 4 years ago

Just saw the last commit, I will look into that as well, thanks!

amotl commented 4 years ago

Dear Mirko,

dwdweather2-0.12.1 should now be compatible with dateutil-2.8.0, which is also given as a minimum dependency, see [1].

With kind regards, Andreas.

[1] https://github.com/panodata/dwdweather2/blob/0.12.1/setup.py#L60

mmaelicke commented 4 years ago

That's super-convenient, thank you Andreas.

amotl commented 4 years ago

The minimum requirement has been bumped to Python 3.3, so I am closing this. Anyone experiencing issues under Python 3, please reopen this.