chore!: replace pytz with zoneinfo #37267
Conversation
openedx/core/djangoapps/content/course_overviews/tests/test_course_overviews.py
Show resolved
Hide resolved
chintanjoshi-apphelix-2u
left a comment
chintanjoshi-apphelix-2u
left a comment
There was a problem hiding this comment.
First round of review is completed.
We can take it up again once those are done.
chintanjoshi-apphelix-2u
left a comment
chintanjoshi-apphelix-2u
left a comment
There was a problem hiding this comment.
Looks good to me, can be sent for further approvals.
chintanjoshi-apphelix-2u
left a comment
chintanjoshi-apphelix-2u
left a comment
There was a problem hiding this comment.
A minor change requested.
| @patch('openedx.core.lib.time_zone_utils.ENABLE_ZONEINFO_TZ') | ||
| def test_mixed_timezone_types_work(self, mock_toggle): | ||
| """Test that mixing pytz and ZoneInfo timezone types works correctly""" | ||
| # Test with ZoneInfo datetime and pytz timezone | ||
| mock_toggle.is_enabled.return_value = True | ||
| zoneinfo_dt = datetime.now(get_utc_timezone()) | ||
| pytz_tz = pytz_timezone('America/New_York') | ||
| result1 = get_time_zone_abbr(pytz_tz, zoneinfo_dt) | ||
| self.assertIsNotNone(result1) # Should not raise an exception | ||
|
|
||
| # Test with pytz datetime and ZoneInfo timezone | ||
| mock_toggle.is_enabled.return_value = False | ||
| pytz_dt = datetime.now(get_utc_timezone()) | ||
| zoneinfo_tz = ZoneInfo('America/New_York') | ||
| result2 = get_time_zone_abbr(zoneinfo_tz, pytz_dt) | ||
| self.assertIsNotNone(result2) # Should not raise an exception |
There was a problem hiding this comment.
DDT can be used here ?
robrap
left a comment
robrap
left a comment
There was a problem hiding this comment.
Minor comments and tests need to be passing. Other than that, let me know when you wish me to approve and merge.
| # .. toggle_use_cases: temporary | ||
| # .. toggle_creation_date: 2025-06-03 | ||
| # .. toggle_target_removal_date: 2025-11-30 | ||
| # .. toggle_tickets: N/A |
There was a problem hiding this comment.
- This is not required, so you can remove it if we aren't going to use it.
- That said, it would be fine to create an issue in edx-platform for removal of this temporary setting to ticket its removal. I won't require that, but that is how this annotation was to be used.
| def test_mixed_timezone_types_work(self, mock_toggle): | ||
| """Test that mixing pytz and ZoneInfo timezone types works correctly""" | ||
| # Test with ZoneInfo datetime and pytz timezone | ||
| mock_toggle.is_enabled.return_value = True | ||
| zoneinfo_dt = datetime.now(get_utc_timezone()) | ||
| pytz_tz = pytz_timezone('America/New_York') | ||
| result1 = get_time_zone_abbr(pytz_tz, zoneinfo_dt) | ||
| self.assertIsNotNone(result1) # Should not raise an exception |
There was a problem hiding this comment.
I appreciate where this is coming from, based on my questions, but I think this test may just add confusion. It would be more clear if you didn't use the toggle and get_utc_timezone(), and just used the implementation code from get_utc_timezone(). This would prove the 2 technologies can be intermixed, but I don't think you actually need to prove it.
Feel free to just remove this test.
If you think this test is really important, then you would also want an assertion for zoneinfo_dt showing it is the type you expect.
|
I'm a bit confused about the PR title and description.
|
|
Closing the PR as i have opened up a new PR #37523 |
3 participants
Description
pytz got deprecated in Django 4.0 && has been completely removed in Django 5.0.
Django 4.2 had provided USE_DEPRECATED_PYTZ flag for pytz support which has now been completely removed in Django 5.0 as well.
Django now uses zoneinfo by default and datetime module use this under the hood now instead of pytz
Difference between datetime.timezone and zoneinfo
datetime.timezone and the zoneinfo package are both related to handling time zones in Python, but they serve slightly different purposes and have different use cases.
datetime.timezone
This is part of the standard library in Python.
It provides a simple way to represent a fixed offset from UTC (Coordinated Universal Time).
It doesn't have information about daylight saving time (DST) or historical changes in time zones.
It's suitable for scenarios where you only need to work with a constant offset, and historical changes in time zones are not important.
zoneinfo
The zoneinfo package is introduced in Python 3.9 as part of PEP 615.
It provides a more comprehensive and accurate way to handle time zones by including historical changes, daylight saving time transitions, and more.
It uses the IANA Time Zone Database, which is regularly updated to reflect changes in time zones around the world.
This package is suitable for applications that require precise handling of time zones, especially when dealing with historical dates.
Original Issue link:
#33980
Related PR:
#37148
Since it involves multiple files changes and as suggested in the PR planning to break that PR into multiple small PR.
Smaller PR link: