Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for Python3 #7

Closed
amotl opened this issue Jun 3, 2019 · 16 comments
Closed

Support for Python3 #7

amotl opened this issue Jun 3, 2019 · 16 comments

Comments

@amotl
Copy link
Member

amotl commented Jun 3, 2019

@Noordsestern asked over at #5

Any plans migrating to python3?

@amotl
Copy link
Member Author

amotl commented Jun 3, 2019

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
Copy link
Member Author

amotl commented Jun 4, 2019

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
Copy link
Contributor

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
Copy link
Member Author

amotl commented Jul 10, 2019

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
Copy link
Member Author

amotl commented Jul 21, 2019

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
Copy link
Contributor

Noordsestern commented Jul 21, 2019

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
Copy link
Member Author

amotl commented Nov 4, 2019

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
Copy link

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
Copy link
Member Author

amotl commented May 30, 2020

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
Copy link
Member Author

amotl commented May 30, 2020

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] dateutil/dateutil#881
[3] https://anaconda.org/anaconda/python-dateutil

@amotl
Copy link
Member Author

amotl commented May 30, 2020

Hi again,

8df0f3d 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
Copy link

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
Copy link

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

@amotl
Copy link
Member Author

amotl commented May 30, 2020

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
Copy link

That's super-convenient, thank you Andreas.

@amotl
Copy link
Member Author

amotl commented Jun 3, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants