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

Add support for datetime with zoneinfo tzinfo #46

Closed
wants to merge 1 commit into from

Conversation

dbkhout
Copy link
Contributor

@dbkhout dbkhout commented Jan 20, 2022

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.

By converting input_datetime (string or datetime object) to a string before applying pandas' to_datetime, compatibility issues between pandas and zoneinfo are circumvented (pandas-dev/pandas#37654).

+ fix typos and remove duplicate code
@stijnvanhoey
Copy link
Contributor

stijnvanhoey commented Jan 21, 2022

@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 #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 Dbkhout datetime py39 #47.

@stijnvanhoey
Copy link
Contributor

closing this one, see #48

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

Successfully merging this pull request may close these issues.

2 participants