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

Fix handling of time series with mixed timezone offsets #68

Merged
merged 4 commits into from
Aug 28, 2024

Conversation

stijnvanhoey
Copy link
Contributor

See #67 for the discussion. This PR handles the request of a custom timezone over a period with mixed timezone offsets. Previously a warning was raised by Pandas and the timestamps were not parsed as pandas Timestamps.

@stijnvanhoey stijnvanhoey requested a review from johanvdw August 16, 2024 09:06
Copy link

Binder 👈 Launch a binder notebook on branch fluves/pywaterinfo/SVH-pandas-warning

@johanvdw
Copy link
Contributor

Seems ok, but just a heads up that we may have to switch away from pytz in the future

https://github.com/stub42/pytz/blob/master/src/README.rst#issues--limitations

This project is in maintenance mode. Projects using Python 3.9 or later are best served by using the timezone functionaly now included in core Python and packages that work with it such as tzdata.

@stijnvanhoey
Copy link
Contributor Author

Seems ok, but just a heads up that we may have to switch away from pytz in the future

https://github.com/stub42/pytz/blob/master/src/README.rst#issues--limitations

This project is in maintenance mode. Projects using Python 3.9 or later are best served by using the timezone functionaly now included in core Python and packages that work with it such as tzdata.

Jup, good point. As we heavily rely on Pandas for the (element-wise) conversion of timezones, we follow the developments on their side. See pandas-dev/pandas#59089 as pytz will be an optional dependency in one of the following releases. Note also that we have some tests already relying on the native timezone implementation (e.g. https://github.com/fluves/pywaterinfo/blob/master/tests/test_waterinfo.py#L307)

@stijnvanhoey
Copy link
Contributor Author

@binomaiheu is it possible that on the current requests to KIWIS all headers get a no-cache property, e.g. 'Cache-Control': 'public,max-age=0,no-cache' which was previously not the case?

@binomaiheu
Copy link
Collaborator

Hmm, we checked, but at first sight, the cache header config of the KiWIS API itself doesn't seem to have changed. Checking with the network admins if this could be due to firewall configuration changes...

@stijnvanhoey
Copy link
Contributor Author

Note that the unit tests are currently failing as I have to request a new HIC token for running the unit tests.

@stijnvanhoey
Copy link
Contributor Author

HIC account update was succesfull. @binomaiheu let me now if the caching on the reverse proxy would been adjusted. I'm merging this PR now

@stijnvanhoey stijnvanhoey merged commit 8d92877 into master Aug 28, 2024
5 checks passed
@stijnvanhoey stijnvanhoey deleted the SVH-pandas-warning branch August 28, 2024 08:52
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.

3 participants