search

fluves / pywaterinfo

Python package to download time series data from waterinfo.be
https://fluves.github.io/pywaterinfo/
MIT License
17 stars 9 forks source link

Add support for datetime with zoneinfo tzinfo #46

Closed dbkhout closed 2 years ago

dbkhout commented 2 years ago

Description

pandas is currently not compatible with the zoneinfo module of the Python standard library (new in version 3.9). This affects pywaterinfo through the use of pandas.to_datetime method in waterinfo._parse_dates.

Example Error

from pywaterinfo import Waterinfo
from datetime import datetime
from zoneinfo import ZoneInfo

dt = datetime(2022, 1, 1, tzinfo=ZoneInfo("Europe/Brussels"))
df = Waterinfo('vmm').get_timeseries_values("78073042", start=dt, end=dt, timezone="Europe/Brussels")

> Exception ignored in: 'pandas._libs.tslibs.conversion._localize_tso'
> Traceback (most recent call last):
> File "pandas\_libs\tslibs\timezones.pyx", line 266, in pandas._libs.tslibs.timezones.get_dst_info
> AttributeError: 'NoneType' object has no attribute 'total_seconds'

Note that a pandas.DataFrame is still returned to df, although it contains unexpected results for this case. This is due to pandas.to_datetime returning a correctly offset (for UTC wall time), but naive timestamp (i.e. 2021-12-31 23:00:00) instead of the expected aware timestamp (i.e. 2022-01-01 00:00:00+01:00). Because of the implementation of waterinfo._parse_dates, this naive timestamp is then localized to the provided time zone, resulting in an incorrect timestamp (i.e. 2021-12-31 23:00:00+01:00).

Proposed Solution

By converting input_datetime (string or datetime.datetime object) to a string before applying pandas.to_datetime, compatibility issues between pandas and zoneinfo are circumvented by relying on the correct implementation of datetime.__str__().

Other

Corrected typos and duplicate code for waterinfo._parse_date.

stijnvanhoey commented 2 years ago

@dbkhout thanks a lot for the very clear issue/pr!

I would like to include the fix. However, I noticed that the PR is originating from your own master branch, which would make your fork not in sync with the master of this repo (also, I could not further extend your branch to add a unit test).

See https://github.com/fluves/pywaterinfo/pull/47 for a new PR with your additions and the unit test. However, as this would not effectively make you a contributor (registered on git/github), I propose we can do either:

  1. you just review the new PR, I add you as a contirbutor to documentation, but no log of git edits.
  2. you create a new branch in you fork, move your commit (e.g. cherry pick) to this new branch and use this one to make a new pull request. If you allow edits by maintainer, I'll add the unit test and we close https://github.com/fluves/pywaterinfo/pull/47.
stijnvanhoey commented 2 years ago

closing this one, see https://github.com/fluves/pywaterinfo/pull/48