Fix tz.gettz() not returning local time for empty string #1024
Conversation
ffe4
force-pushed
the
issue_926
branch
2 times, most recently
from
a6f8698
to
841cc6c
Compare
ffe4
changed the title
ffe4
force-pushed
the
issue_926
branch
2 times, most recently
from
14d4328
to
167bcf0
Compare
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
ffe4
force-pushed
the
issue_926
branch
3 times, most recently
from
492334b
to
df29e69
Compare
| @@ -1596,7 +1596,7 @@ def nocache(name=None): | |||
| 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.
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