-
-
Notifications
You must be signed in to change notification settings - Fork 19.4k
DEP: Remove tzdata as a hard dependency outside of Windows #63335
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
base: main
Are you sure you want to change the base?
Conversation
| "numpy>=1.26.0", | ||
| "python-dateutil>=2.8.2", | ||
| "tzdata>=2023.3" | ||
| "tzdata>=2023.3; platform_system == 'Windows'", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With tests seeming to be fine with no minimum here (ref: #63264 (comment)), I'm wondering if we should remove the lower bound.
|
No objection here. I didn't follow the conversation super-closely though. |
|
Just noting that the conda-foge recipe should also be updated to make move |
| - [NumPy - Adds support for large, multi-dimensional arrays, matrices and high-level mathematical functions to operate on these arrays](https://www.numpy.org) | ||
| - [python-dateutil - Provides powerful extensions to the standard datetime module](https://dateutil.readthedocs.io/en/stable/index.html) | ||
| - [tzdata - Provides an IANA time zone database](https://tzdata.readthedocs.io/en/latest/) | ||
| - [tzdata - Provides an IANA time zone database](https://tzdata.readthedocs.io/en/latest/) (Only required on Windows) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or emscriptem?
|
I am fine with this, and also fine with removing the lower bound on the version. Do you know what kind of error you now would get on Windows if you don't have tzdata installed? (I don't know if the |
Dr-Irv
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This probably will require a change to the pandas-feedstock for conda
|
I added a test for the error message; a bit hacky, can remove if people prefer. |
| "tzdata>=2023.3" | ||
| "tzdata>=2023.3; platform_system == 'Windows'", | ||
| # Emscripten is the platform system for Pyodide. | ||
| "tzdata>=2023.3; platform_system == 'Emscripten'", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see some other packages use sys_platform != "emscripten" instead. Do we know if there is a preference? (I don't directly find a good source about this in the docs)
tzdataPython package #63264 (Replace xxxx with the GitHub issue number)doc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature.AGENTS.md.This may still need more discussion.
Docs change: