Handle ZoneInfo objects in get_timezone_location, get_timezone_name#741
Conversation
…llow it. Run the tests for zoneinfo only if available
…ividual cases fail
Codecov Report
@@ Coverage Diff @@
## master #741 +/- ##
==========================================
+ Coverage 90.99% 91.02% +0.02%
==========================================
Files 24 24
Lines 4186 4188 +2
==========================================
+ Hits 3809 3812 +3
+ Misses 377 376 -1
Continue to review full report at Codecov.
|
| @pytest.mark.parametrize("timezone_getter", ["zoneinfo.ZoneInfo"], indirect=True) | ||
| def test_get_timezone_name_time_zoneinfo(timezone_getter, tzname, params, expected): | ||
| """zoneinfo will correctly detect DST from the object. | ||
| FIXME: this test will only succeed in the winter. |
There was a problem hiding this comment.
how should we address this? Should I just test with a full datetime object instead?
There was a problem hiding this comment.
I... guess? Getting a timezone from just a time-of-day seems weird anyway.
There was a problem hiding this comment.
Shall I just delete this test case?
Alternatively, I can try patching datetime.utcnow() to return a constant datetime, and test against that. But it seems that the datetime module is not patchable: https://stackoverflow.com/a/192857/714760. So it would be really hard to test this case.
There was a problem hiding this comment.
I will remove this test.
|
@akx could you have a look at this? |
| if hasattr(tzinfo, 'zone'): # pytz object | ||
| zone = tzinfo.zone | ||
| elif hasattr(tzinfo, 'key') and tzinfo.key is not None: # ZoneInfo object | ||
| zone = tzinfo.key | ||
| else: | ||
| zone = tzinfo.tzname(dt or datetime.utcnow()) |
There was a problem hiding this comment.
This stanza seems to be more or less the one in the below change fragment – could they be refactored into one function like get_tz_name(tzinfo, default=None)?
| @pytest.mark.parametrize("timezone_getter", ["zoneinfo.ZoneInfo"], indirect=True) | ||
| def test_get_timezone_name_time_zoneinfo(timezone_getter, tzname, params, expected): | ||
| """zoneinfo will correctly detect DST from the object. | ||
| FIXME: this test will only succeed in the winter. |
There was a problem hiding this comment.
I... guess? Getting a timezone from just a time-of-day seems weird anyway.
|
@youtux, re:
Regarding this, #569 is probably still relevant. I'm afraid a lot of old code and systems rely on "just give me some Babel, I don't care if it's compatible", and if a release drops support for an older Python, breakage could be immense. I do agree we should drop support for EOL versions of Python, I just don't know what the graceful way to do it is. |
It should not be a problem if the new release will correctly declare which python versions are supported by using the python_requires declaration in the setup.py/setup.cfg. |
|
@youtux Yeah, good point. That should work (assuming, you know, that people install packages with a modern I think we should still have one more release with the current guaranteed compatibility, just to ship e.g. the new CLDR changes. |
You can't save everyone :) |
|
I know, but a package that still has 26,000 daily downloads on Py2 should take care of those users at least somewhat! :) |
|
I'm not sure it should. Popular projects like django and pytest already dropped support for python2 completely. |
3 participants
Python 3.9 introduced ZoneInfo objects, and they are available as a backport back to python 3.6.
This PR adds support for these objects.
Fixes #740