Repository navigation
Backport zoneinfo logic into dateutil.tz.tzfile - #1130
Conversation
|
@pganssle do you want me to review this? |
|
@mariocj89 I still need to backport the tests, but you are welcome to review it. I expect in the long run we'll want to consolidate some of the logic around Also I believe some documentation changes will be necessary before we can merge this as well. |
|
OK, I'll take a look once all is ready. |
|
So @mariocj89 I suppose this is really the only remaining blocker for dropping support for Python 2? Dropping support for Python 2 would mean we could get into in-place typing info and all the new fun stuff right :)? Given this has been open for ages, should I try to prioritize this MR, open a new MR and see if we can get somewhere within the next few months? |
|
+1 |
|
Hi @mariocj89,
|
Without this change users in 2.7 will not get tz upgrades. This splits the tzdata from the package, allowing users to stay in an old version of the package but still get tzdata upgrades.
Correct
Needs to work for py2, so no type hints :). |
Now it completely makes sense! Thanks. |
|
@mariocj89 to improve visibility on the progress & smoothen the process a little. Can we open a new branch containing this work so far? I was diving a bit further (as I want to make sure I understand the full context before making changes) and was wondering the following: |
|
@pganssle are you still actively working on getting this change merged? |
|
Unfortunately I don't have the time anymore to work on this and made zero progress anyway. So I don't think anyone is working on this. The code is very complex and you are only doing this for python 2 users. Given that reached end of life years ago, I'm not to enthusiastic to work on this anyway. |
24c8f99 to
4d4bafa
Compare
06dd279 to
e362489
Compare
On Python 3.9+ dateutil.tz._tzpath just re-exports the standard library's search path handling, so the backport for older versions was only ever exercised on the 3.5-3.8 CI jobs, and then only indirectly. These tests execute the module source with zoneinfo blocked so the backport is tested on every version: the platform defaults, parsing of PYTHONTZPATH including the warning for relative entries, the TypeError for a bare string and ValueError for relative paths in reset_tzpath, path-like arguments, and the callbacks used to keep dateutil.tz.TZPATH current on Python < 3.7. A separate test checks that dateutil.tz.TZPATH itself follows reset_tzpath, which is what the missing callback registration broke.
In a zone with exactly one transition and no DST rule in the footer, `fromutc` compared the offset after the transition against itself when detecting a fold, so it never set `fold=1`. For a transition that moves the offset backwards (e.g. Antarctica/Rothera in 1976, or Africa/Maputo and its links in 1908), the times in the fold came back with `fold=0`, and converting them back to UTC was off by the size of the shift. The branch that handles this case can only be reached when there is a single transition, so the offset before the transition is always `_tti_before`. This bug is inherited from the pure-Python implementation of `zoneinfo`; the C implementation gets it right. `test_weirdzone_utc` compared two datetimes with the same `tzinfo`, which ignores `fold`, so it now also checks the round trip back to UTC and the value of `fold` for ambiguous times.
Python < 3.6 does not allow tzinfo offsets that are not a whole number of minutes, so tzfile rounds them there while keeping the raw values for pickling. The parsing side of this was covered by the sub-minute fixes earlier in this branch, but nothing checked that the raw offsets actually survive a pickle round trip, which is what allows a pickle made on an old version to load exactly on a new one.
Beyond the import-time deprecation warnings, nothing checked what the rewritten dateutil.zoneinfo actually does: that ZoneInfoFile() loads every zone from tzdata with the key set and reports tzdata's version as its metadata, that it raises a useful ImportError without tzdata, that the legacy tarball loader still works (it was being overwritten by the tzdata load), and that the deprecated gettz goes through it.
The caching wrapper around gettz has a lock, but nothing exercised it with the new tzfile, which does its own file parsing on a cache miss. This runs gettz from several threads while another thread clears the cache, and checks that every call returns a zone equal to the one from a single-threaded call.
After the last explicit transition in the file, `fromutc` hands off to the TZ string, which only detects folds created by its own DST rules. If the last explicit transition itself moves the offset backwards, the times in the fold it creates came back with `fold=0`, and converting them back to UTC was off by the size of the shift. This mostly shows up with "slim" TZif files like the ones in the tzdata package, where the last explicit transition is often a change of standard offset followed directly by the TZ string. For example, with the tzdata package, `datetime(2022, 11, 30, 6, 30, tzinfo=UTC)` converted to America/Ciudad_Juarez gave 23:30-06:00 rather than 23:30-07:00 with `fold=1`. The "fat" files on most systems have explicit transitions up to 2037, so they are not affected in practice. As with the single-transition case, this is inherited from the pure-Python implementation of `zoneinfo`; the C implementation gets it right, and with this change `fromutc` matches it for every zone in the tzdata package.
`rebuild` compiled a tzdata release with `zic` and wrote the result into the `dateutil.zoneinfo` package as the bundled tarball. There is no tarball any more: `ZONEFILENAME` is `None`, so `rebuild` failed with a `TypeError` after running `zic`, and nothing would have read its output anyway. It now does nothing except raise `RebuildDeprecationWarning`. Callers that relied on it are almost certainly doing something that no longer has any effect, so a warnings filter turns this into an error by default. The filter is appended at the end of the filter list, so users who just want the old call to keep working can ignore the warning with a filter of their own. The warning is a `FutureWarning` rather than a `DeprecationWarning`, because the default filters ignore `DeprecationWarning` before an appended filter is ever reached.
The `tz` environment builds the tz database from its master branch and points PYTHONTZPATH at the result. Now that dateutil depends on tzdata, any key that is missing from (or cannot be read in) that build is looked up in the tzdata package instead, and even a wrong PYTHONTZPATH makes every test pass against tzdata. Uninstall tzdata before running the tests, and check that TZPATH is the directory we just built and that a zone is actually loaded from it before going any further. The tests that need tzdata are skipped.
A TZif file cut off inside the header or one of the data tables raised `struct.error` from `tzfile`, where a file cut off inside the footer raises `ValueError`. `gettz` only skips invalid files on TZPATH that raise `ValueError` (or `OSError`), so a truncated file there made `gettz` fail rather than moving on to the next entry. CPython's `zoneinfo` also raises `struct.error` here, so this is not a regression, but there is no reason to report the same problem in two different ways.
`gettz` joins a relative key onto each TZPATH entry, so a key like `"../../etc/localtime"` loads a file from anywhere on the file system. This is not a big problem, since the file still has to be a valid TZif file, but it's more lenient than it needs to be for anyone passing untrusted input to `gettz`, and CPython's `zoneinfo` rejects these keys. These keys now raise `DeprecatedTzKeyWarning`. As with `dateutil.zoneinfo.rebuild`, a warnings filter appended at call time makes this an error by default, and users who need the old behavior for now can filter the warning out. Keys that stay inside the search path once normalized (e.g. `"America/../UTC"`) are not affected, and neither are absolute paths.
`tzfile._is_ambiguous_or_imaginary` returns early for fixed-offset zones and for `None`, which nothing in the test suite was exercising.
The `tz` job only built tz master with `zic -b fat`, but `-b slim` has been zic's default for a while and is what most distributions (and the tzdata package) ship. Slim files have an empty version 1 data block and rely on the TZ string footer for everything after the last transition, which exercises different code paths in the parser. This splits the job into `tz-fat` and `tz-slim`, and checks that the installed data really is fat or slim before running the tests.
`construct_zone` never wrote leap second records or the standard/wall and UT/local indicators, so nothing tested that the parser skips over them correctly, in particular that leap second records are 8 bytes in the version 1 data block and 12 bytes in the version 2+ block. It also wrote those sections in the wrong order, which only didn't matter because they were always empty. It can now write both, and the new test checks that they don't change how a zone behaves for versions 1 through 4, including a version 4 style leap second table whose start has been truncated.
The rules in a TZ string are in terms of the local year, and a rule can put a transition just past the end of its year (e.g. "J365/25" is 1:00 on January 1st of the next year). `fromutc` only looked at the rules for the UTC year, and `utcoffset` only at those for the local year, so near the new year, both could miss a transition that comes from the previous or next year's rules. The idiom zic uses for "DST all year" depends on exactly this: each year's DST period ends at the same instant as (or after) the next one starts. The rearguard data (and so presumably macOS) describes Africa/Casablanca after 2087 as "XXX-2<+01>-1,0/0,J365/23"; the same rule with "J1/0" in place of "0/0" is what the tests use, since "0/0" has a separate problem, fixed in the next commit. Looking only at the UTC year, `fromutc` put the last two hours of every year in standard time, so those times didn't round trip. Within ten days of the new year, `fromutc` now builds the list of transitions from the rules for the year before, the year itself and the year after, and uses the last one before the time in question. `utcoffset` and friends answer the question in terms of UTC in the same window: a local time is valid for an offset if `fromutc` would give that offset at the corresponding UTC time. Everywhere else the existing single-year logic is unchanged, which is both equivalent and faster. CPython's `zoneinfo` (C and pure-Python) has the same problem.
`_post_epoch_days_before_year` returned one day less than its docstring says, which the "Mm.w.d" rules and 1-based "Jn" rules made up for by adding their 1-based day of the month or year. Zero-based "n" rules don't, so they came out a day early: "10/0" was January 10th rather than 11th, and "0/0" was December 31st of the previous year. Separately, "Jn" rules never count February 29th, so in leap years the days from J60 (March 1st) on are one day later than their number suggests, but the adjustment started at J59, which put J59 on February 29th rather than 28th. This makes `_post_epoch_days_before_year` do what it says, and fixes both rules; the expected dates in the tests were checked against glibc. zic uses "0/0" for zones that are on DST all year, so this also matters for real data, e.g. Africa/Casablanca after 2087 in the rearguard format. Both bugs are inherited from CPython's `zoneinfo` (C and pure-Python).
For a local time after the last explicit transition, `utcoffset` and friends use the TZ string. But the TZ string only describes times after that transition, and its answer for a given local time can correspond to a UTC time before it. When that happens, the explicit transitions are what apply, so use the offset from the last one instead. This happens with the slim America/Nuuk file in the tzdata package. Greenland stayed on -02 through the end of 2023, and the last explicit transition, at 2023-10-29 01:00 UTC, is a change from DST to standard time at the same offset. The TZ string, `<-02>2<-01>,M3.5.0/-1,M10.5.0/0`, would have DST end at midnight local time that night, so for wall times between 23:00 and midnight it gave -01, which puts them before the last transition. Those times didn't round trip, and `datetime_exists` said they didn't exist. CPython's `zoneinfo` (C and pure-Python) has the same problem.
`gettz` falls back to the tzdata package for any key that is missing from, or can't be read on, TZPATH. That's the right behavior, but it means that the tests that are about the system time zone data (or the pinned or tz master data CI installs there) can't tell whether they tested that data at all: with an empty TZPATH and tzdata installed, the whole test suite passed. Rather than uninstalling tzdata, which dateutil depends on, the tests that are about the data on TZPATH now run against each source of data in turn: - "tzpath": the data on TZPATH, with tzdata blocked in-process, so a key that is missing or broken there fails the test. These are skipped when there is no data on TZPATH at all (e.g. on Windows). - "tzdata": the tzdata package, with an empty TZPATH. This is the `tz_source` fixture for pytest tests, and `TzSourceMixin` for unittest test cases, which can't use parametrized fixtures, so the affected test cases (`GettzTest`, `DatetimeAmbiguousTest`, `DatetimeExistsTest`, `ImaginaryDateTest`) have a subclass for tzdata. `set_tzpath` and the helpers it uses move from `test_tzfile.py` to `tests/_common.py` so everything can use them, and the lock they take is now reentrant so a test running under the fixture can still use `set_tzpath` itself. Along the way: - Time zones that were looked up at import time (in `test_utils.py`, `test_isoparse_prop.py` and the `resolve_imaginary` tests) are now looked up in the tests, or checked, so that a missing key fails rather than quietly turning into `None`. - Tests that used links from the "backward" file for convenience use the zones they point to instead, since some builds of the tz database leave those links out. A new test checks that the links resolve to the same data as their targets, and is skipped for links that aren't there. - There are new tests for what `gettz` does when a file on TZPATH is invalid (it moves on to the next entry, and then to tzdata) and when there's no data anywhere (the `UTC`/`GMT` and `time.tzname` fallbacks). - The tz master job no longer uninstalls tzdata; the tests that need to be protected from it now protect themselves.
The transition tests in test_tzfile.py pin their data so that they can check specific historical transitions, which means that in the tz master CI jobs they never see the data being tested. These tests instead check properties that must hold for any valid time zone data, for every key that is available, against both the data on TZPATH (with tzdata blocked) and the tzdata package: - every key loads, and from TZPATH when it should; - converting from UTC to local time and back gives the same instant; - `datetime_ambiguous` and `datetime_exists` agree with what the two values of `fold` give for `utcoffset()`; - on Python 3.9+, `fromutc`, `utcoffset`, `dst` and `tzname` agree with the C implementation of `zoneinfo`. They sample fixed dates from 1850 to 9000, the first and last explicit transitions of each zone, and the transitions generated by the TZ string in 2040 and 2100, and take a few seconds for the ~600 zones. The ambiguity check makes an exception for one window in America/Nuuk: in slim builds, its last explicit transition is followed by a TZ string that disagrees with it for an hour, so `utcoffset()` and `fromutc()` disagree there. CPython's `zoneinfo` does the same.
TZif files record the total UTC offset of each local time type and whether it is DST, but not how much of the offset is DST, so `dst()` is inferred from the neighboring transitions. That inference is a heuristic, and the other tests of `dst()` either use hard-coded historical values or compare against CPython's `zoneinfo`, which uses the same heuristic, so neither would notice it going wrong with new data from upstream. The `tzdata.zi` file that the tz Makefile installs next to the TZif files (and that the tzdata package ships) has the standard offset for every period of every zone, so away from the boundaries between periods `utcoffset() - dst()` has to be the standard offset from there. That doesn't require evaluating any of the DST rules, only the zone lines. The heuristic currently gets this wrong for 29 zones, including British Double Summer Time in Europe/London, half-hour DST in Pacific/Rarotonga and America/Montevideo, and zones like America/Bahia_Banderas where the standard offset changed at the same time as DST started. Those are listed as known failures (the same list covers the 2024a and 2025b releases and tz master, in every data form, as fat and slim files), so that only new failures fail the test.
The tz database source is published in three data forms. "main" is the default, "vanguard" uses newer zic features before they become the default, and "rearguard" avoids features that some consumers can't handle yet, most visibly negative DST (e.g. Europe/Dublin's TZ string is `IST-1GMT0,M10.5.0,M3.5.0/1` in main and `GMT0IST,M3.5.0/1,M10.5.0` in rearguard). Distributions ship all of them, but the tz jobs only built the main form. This adds `tz-vanguard` and `tz-rearguard` jobs, built with zic's default of slim TZif files, and has the script check that the installed data really is in the expected data form (vanguard and rearguard builds of `tzdata.zi` have a "# dataform" line after the version).
The ported heuristic assigns one DST offset to each local time type, taken from the standard time just before the first transition to it. Against the standard offsets in tzdata.zi that gets 26 zones wrong. Bahia_Banderas, Inuvik, Santiago and Ust-Nera are regressions from the old dateutil heuristic, and it is worse for Rarotonga and Montevideo. It fails in three situations: - zic reuses one local time type for periods with different standard offsets (e.g. UTC+0 DST in the Azores was double summer time in the 1940s and summer time later), and one value per type can only be right for one of them. - Double summer time follows another DST type, so the offset before it is not standard time. - When the standard offset changes at the same time as a DST transition (Bahia_Banderas in 2010, Rarotonga in 1978), the standard time before the DST period implies the wrong amount. This infers dst() for each transition instead, from the last standard time before and the first one after each DST period. When the two disagree, it prefers an amount that is a multiple of 5 minutes (a change from LMT at the end of DST can imply 0:34:39), then one the zone used unambiguously within ten years (Tehran 1977 and Rarotonga 1978 look the same in TZif, but used 1:00 and 0:30), then positive DST (the 1942 occupation zones imply -1:00 or +1:00), then the smaller amount. Each of these is needed for at least one zone. On the 2024a, 2025b and 2026e releases and tz master in the main, vanguard and rearguard forms, as fat and slim files, the only remaining errors are three samples in Europe/Paris from the liberation to September 1945. The TZif data there looks the same as the CET period that followed, so I've left it as a known failure, limited to those dates. Since this no longer matches CPython's zoneinfo (which still has the old heuristic; see gh-85102), the comparison with zoneinfo no longer checks dst(), and tzdata.zi is the reference for it. Because the transitions no longer map back to local time types by identity, the raw type indices and isdst flags are kept for pickling.
|
OK, after 5 years I believe this is ready to merge. |
Summary of changes
This moves
tzfileinto its own submodule containing what is effectively a backport of the pure python implementation ofzoneinfo.ZoneInfo, but with the semantics of adateutil.tz.tzfileobject.The major differences from zoneinfo are:
tz.gettzrather than intz.tzfile.tzfileobjects are pickled by value, not by reference to the IANA key.tzfilehas equality-by-value semantics, whereaszoneinfohas equality-by-identity semantics.This does change the internal implementation details of the class. It also fixes at least two bugs:
dateutildid not use thefoldattribute during gaps (only during folds), in violation of PEP 495. This fixes that, but that does change the semantics of imaginary times a bit (and thus may affect some mechanisms for detecting imaginary times).This is mostly a code dump with compatibility adjustments. It's quite possible that we'll want to refactor, particularly with respect to the way POSIX strings are handled. Theoretically
tzfilecould fall back to atz.tzstror some othertz.tzrange, or we could refactortzstrandtzrangein terms of _CalendarOffset, _DayOffset and _TZStr (or both, by refactoring the internal logic oftzstrand/ortzrange).Closes #462, #1059
Open Tasks:
keyparameter for consistency withzoneinfo.ZoneInfoTZPATHtzdatablack(at least trying to be incremental here when we can...)Pull Request Checklist