DEPS/TST: tzdata is optional, not required #47467
Conversation
|
OK, this works now. We should probably specify a minimum tzdata version too, though. @jbrockmendel Do you have anything in mind? |
|
no idea, cc @pganssle ? |
|
seems fine, does this need backport? |
jreback
added
Timezones
My 2c is might as well have the min version be the latest available version since this is a new feature.
Don't think so since |
correct. the failures were only on nightlies (main). 1.4.x is passing tests (or was when I last run the build test which is running again now ready for release MacPython/pandas-wheels#173) |
|
Zoneinfo is also kinda wonky, since it doesn't really need the tzdata package, just a copy of the IANA tz database. I'm not sure we have a way to check the version of the IANA tz database, though. |
I don't think we should necessarily runtime check |
|
This is blocking #47442, so we should probably merge this now, and open an issue to discuss tzdata version, since there is not yet enough consensus on that. |
|
IMO setting |
Co-authored-by: Matthew Roeschke <emailformattr@gmail.com>
…as into fix-tzdata-checking
pandas/_libs/tslibs/timezones.pyx
Outdated
| try: | ||
| # py39+ | ||
| import zoneinfo | ||
| from zoneinfo import ZoneInfo | ||
| import_optional_dependency("tzdata", errors="raise", min_version="2022.1") |
There was a problem hiding this comment.
IIRC we should only check tzdata if the user doesn't have an OS that already has system timezones that are available.
import zoneinfo
from zoneinfo import ZoneInfo
try:
ZoneInfo("UTC")
except zoneinfo.ZoneInfoNotFoundError:
import_optional_dependency("tzdata", errors="warn", min_version="2022.1")
Also I think errors="warn" since tzdata is still optional, and the user may not necessarily use ZoneInfo objects when using pandas (since it's not the default yet)
…as into fix-tzdata-checking
|
OK, so I've updated this PR again. It's too difficult/costly to check the version of the system IANA tz db if present, so we're just going to specify a minimum version. If tzdata is installed, though, the minimum version will be enforced through a warning, even if they have an up to date IANA tz db, since we would want to warn them of the mismatch. |
|
Hello @lithomas1! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2022-08-09 13:55:50 UTC |
| Dependency Minimum Version Notes | ||
| ========================= ========================= ============================================================= | ||
| tzdata 2022.1(pypi)/ Allows the use of ``zoneinfo`` timezones with pandas. | ||
| 2022a(for system tzdata) **Note**: You only need to install the pypi package, if your |
There was a problem hiding this comment.
Looks like the 2022a is a conda forge package?
https://anaconda.org/conda-forge/tzdata
https://github.com/eggert/tz
Which is a different project from tzdata (2022.1).
https://pypi.org/project/tzdata/#history
https://github.com/python/tzdata
I'm not sure it's a good idea to recommend 2 differently maintained packages here. Can we just recommend the pypi one since it's the once recommended in the official docs https://docs.python.org/3/library/zoneinfo.html#data-sources?
There was a problem hiding this comment.
I think we'd still want to state a min version for system tzdata for consistency. 2022a is also the version of the actual IANA tz db(it's python's tzdata versioning scheme that's different).
There was a problem hiding this comment.
Ah okay sounds reasonable then. Might be good then to mention that the system tzdata can be updated by installing the conda forge package?
|
I'm going to self merge this in a couple of days if no other comments, so we can get the Python 3.11 testing in. |
|
thanks @lithomas1 this is great |
doc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature.