jim-easterbrook / pywws

Python software for USB Wireless WeatherStations
https://pywws.readthedocs.io/
GNU General Public License v2.0
204 stars 62 forks source link

pytz dependancy but not explicitly listed #102

Open jarvisms opened 3 years ago

jarvisms commented 3 years ago

pytz isn't actually listed as an immediate dependency of pywws in setup.py even though it is imported directly in timezone.py. Instead, it's a dependency of the current version of tzlocal.

However looking at tzlocal on github, they removed the pytz here: https://github.com/regebro/tzlocal/commit/ee1354ff97a70e9b0f1649d32010f64133b88aed. This isn't the main release but may be in the future in which case then, pywws may break.

Either pytz needs to be an explicity dependancy or pytz needs to be written out and tzlocal used exclusively.

jim-easterbrook commented 3 years ago

Thanks. The whole mess of timezone computations needs an overhaul anyway. I'll add the pytz dependency as part of that.

jarvisms commented 3 years ago

Timezones and summer time in particular is very irritating - especially when you assume a day is 24 hours but then it's 23 or 25 as summertime changes!

In other projects, I try to strictly store/process as UTC (tz aware datetime objects) and only handle tz naive datetimes when accepting inputs from users/templates or presenting back to a user and in those cases, I assume localtime unless its explicit. I've never used libraries like pytz, just native python datetime. I'm not familiar with timezone.py and what it does but I'm happy to support, review or test stuff.

jim-easterbrook commented 3 years ago

I agree. pywws uses UTC for all time stamps etc, with conversions to/from local time where needed.

Things get a bit more complicated with things like graph axes, where gnuplot needs a continuous sequence of time stamps in local time but the offset from UTC changes as the graph spans the clock change.

jim-easterbrook commented 3 years ago

Looking at that commit to tzlocal it seems that they're dropping pytz in favour of zoneinfo, which appears to be backported from Python 3.9. I'm only using pytz directly for the pywws.timezone test routines, as these need to use explicit time zones rather than the user's local time zone.

jarvisms commented 3 years ago

As yes, I see it only appears twice. So could the pytz bits just be switched for datetime.tzinfo stuff, or equivelent from tzlocal and be done with it?

jim-easterbrook commented 3 years ago

I'm going to try and strip out everything pywws doesn't need, then decide what's needed for a useful test routine. tzlocal is the simplest way to get the user's time zone, and returns a pytz tzinfo which should be compatible with datetime's tzinfo. If I can stick to the datetime API then everything should work now and in future.

jarvisms commented 3 years ago

The quick and dirty way I capture the system local timezone is with datetime.datetime.now().astimezone().tzinfo but I think that only works on Python 3.6 onwards. With a tweak it can work on 3.3 I believe but further back then that, I don't have any one liners up my sleeve.

jim-easterbrook commented 3 years ago

Yes, a lot of the reasons for using tzlocal and pytz disappear with PEP 495 which is implemented in Python 3.6 and after. https://www.python.org/dev/peps/pep-0495/

ashenshugarRET commented 3 years ago

Hi, Not sure if this is helpful, but it appears the issue is only in the handling of text templates, as my equivalent GNUPlot chart seems to be rendering fine with version 21.3.0: image

I've just moved away from these fixed graphics to working with JSON templated data for Highcharts for the additional interactive features... image

jarvisms commented 3 years ago

Those charts dont actually have the summer time change in which was 1am-2am on Sunday 28th 2021 (Last sunday of March). In other work, sometimes I found that spring change is OK but the autumn change (last sunday of October) is not because an hour is almost duplicated so best to check both ends work (and show the data you expect at the right time).

ashenshugarRET commented 3 years ago

Sorry,

I should have explained better. The two charts above are generated using scripts which are supposed to jump to midnight of the current day to produce summaries for today, using pywws v21.3.0...

The command within the text template: `#timezone local#

roundtime True

goto "%Y-%m-%d 00:00:00"#`

Is actually jumping to 00:00:00 UTC not BST so the chart starts at 01:00:00 BST when I use v21.3.0, interestingly it'll jump to 00:00:00 BST with v20.1.0.

Interestingly the command to jump to midnight in the graph template: <start>hour=0</start> Jumps to 00:00:00 BST with both v20.1.0 and v21.3.0

jim-easterbrook commented 3 years ago

Commit 952dd9e switches everything over to using a new time_zone object introduced in commit ad58ca0. If anyone wants to test this (particularly if you live a long way from Greenwich) then please do.

ashenshugarRET commented 3 years ago

Dear Jim,

I've setup pywws from the GitHub repository on a test Raspberry Pi. I can confirm that my text template which jumps to 00:00:00 local time to produce a summary of the days's weather readings works as intended and does not replicate the error of jumping to UTC, although I am based quite close to Greenwich...

I've also confirm that pywws.timezone runs as expected from the CLI but did still need pytz installed to run.

I've switched my production pywws Pi over to running release versions of pywws from PiP so I can't easily fully test Github builds anymore, although I hope this limited testing is helpful for you.

jim-easterbrook commented 3 years ago

I haven't removed the old timezone stuff yet, just in case I need a fallback. Eventually the only dependency on pytz will be via tzlocal, if using a version before 3.0.