Fix tz.gettz() not returning local time for empty string#1024
Conversation
a6f8698 to
841cc6c
Compare
ffe4
changed the title
14d4328 to
167bcf0
Compare
pganssle
left a comment
pganssle
left a comment
There was a problem hiding this comment.
This is turning out trickier than I expected. Thanks for putting in the work on this @ffe4!
One additional comment: I've been putting property tests in their own file in the tests/property directory to make it easier to run them separately from the main test suite. Can we move the property test into a new tests/property/test_tz_prop.py file?
ffe4
changed the title
492334b to
df29e69
Compare
| name = os.environ["TZ"] | ||
| except KeyError: | ||
| pass | ||
| if name is None or name == ":": |
There was a problem hiding this comment.
I'll note that this will suddenly allow tz.gettz(0) without throwing an exception.
Maybe we should go with if name is None or name in ("", ":"):?
In a potential future version we may want to explicitly raise TypeError if name is not an instance of str or None, but I'm inclined to defer the discussion about duck typing/leniency and just implement this in such a way that we don't have to address it right now.
There was a problem hiding this comment.
There was a problem hiding this comment.
I personally rewrite my history so that each commit is an atomic change where the tests pass. If that's too much of a pain, squashing it down is also a valid approach.
The documented behavior of the function is to return a local time zone when the argument is None or an empty string. This was working if the TZ environment variable was set, but not working otherwise. Fixes dateutil#925, dateutil#926
The current tests only check that `tz.gettz()` and `tz.gettz("")` do
the same thing, but not that the thing that they do is equivalent to a
local time zone.
Defining what is a "local time" is a bit complicated, considering that
`tz.gettz()` may return a `tzfile` or a `tzlocal`, and neither of those
have the same behavior as Python's built-in local time support via
`.astimezone()`. This property test checks only the datetimes and
properties that are expected to be the same between the various methods
of getting a "local" time zone.
2 participants
Summary of changes
tz.gettz()now properly checks for empty string and returns local timetz.gettz()returning the same result when passed empty string orNonenocachechecks thegettz(name)parameter withif not nameand tries to get the local time from theTZenvironment variable. When the environment variable does not exist, it only checked forname is None, skipping the logic that searches for the local timetzfilewhen passed an empty string.Closes #925
Closes #926
Pull Request Checklist