-
Notifications
You must be signed in to change notification settings - Fork 95
Address some warnings #6640
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Address some warnings #6640
Conversation
|
@hjoliver, this PR removes a code block that isn't used any more. Can you have a quick look at it and merge if ok. |
|
@hjoliver ping |
| if custom_time_zone_info is not None: | ||
| custom_hours = custom_time_zone_info["hours"] | ||
| custom_minutes = custom_time_zone_info["minutes"] | ||
| if use_basic_format: | ||
| custom_string = custom_time_zone_info["string_basic"] | ||
| else: | ||
| custom_string = custom_time_zone_info["string_extended"] | ||
| if date_time_is_local: | ||
| date_time_hours = TIME_ZONE_LOCAL_UTC_OFFSET_HOURS | ||
| date_time_minutes = TIME_ZONE_LOCAL_UTC_OFFSET_MINUTES | ||
| else: | ||
| date_time_hours, date_time_minutes = (0, 0) | ||
| diff_hours = custom_hours - date_time_hours | ||
| diff_minutes = custom_minutes - date_time_minutes | ||
| date_time = date_time + timedelta( | ||
| hours=diff_hours, minutes=diff_minutes) | ||
| time_zone_string = custom_string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hjoliver, this PR removes a code block that isn't used any more. Can you have a quick look at it and merge if ok.
This code block was previously only used by the Cylc 7 GUI (7.8.15):
cylc-flow/lib/cylc/gui/updater_tree.py
Lines 269 to 277 in e60e8f7
| # Task not finished, but has started and has a meant; | |
| # so we can compute an expected time of completion. | |
| tetc_string = ( | |
| self._id_tetc_cache.get(id_, {}).get(tetc_unix)) | |
| if tetc_string is None: | |
| # We have to calculate it. | |
| tetc_string = get_time_string_from_unix_time( | |
| tetc_unix, | |
| custom_time_zone_info=time_zone_info) |
However, even then it looks like it was only used for the same purpose that is already provided by override_use_utc:
cylc-flow/lib/cylc/gui/updater_tree.py
Line 190 in e60e8f7
| time_zone_info = self.updater.global_summary.get("time zone info") |
cylc-flow/lib/cylc/state_summary_mgr.py
Lines 125 to 128 in e60e8f7
| if get_utc_mode(): | |
| global_summary['time zone info'] = TIME_ZONE_UTC_INFO | |
| else: | |
| global_summary['time zone info'] = TIME_ZONE_LOCAL_INFO |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hjoliver poke
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hjoliver poke
|
(bumped to 8.5.0 as this is non-critical, bump back if merged in time) |
|
Python 3.15 is out next year, hence I think this ought to go in soon (8.7.0 at the latest) |
Spotted in pytest output
datetime.utcnow()Note
There is a risk with this because using comparison operators (e.g.
<) between timezone-aware and naive (timezone-unaware)datetimes results in a type error. However I cannot find any instances of doing any such comparisons, and all the tests are passing.sqlite3.connect():Check List
CONTRIBUTING.mdand added my name as a Code Contributor.?.?.xbranch.