update test_time and implement more time module#7073

youknowone

Summary by CodeRabbit

Bug Fixes
- Enhanced time module functions with improved platform compatibility and reliability across Unix and non-Unix systems.
- Strengthened time conversion and structure handling for more accurate operations across different operating environments.
Refactor
- Refined internal time handling logic to provide consistent behavior and better cross-platform support.

coderabbitai

📝 Walkthrough

Walkthrough

The changes extend the Unix timestamp and time structure handling in the time module with platform-specific code paths and new helper functions. Function signatures for gmtime, localtime, and ctime now accept optional nullable time values. StructTimeData gains new constructors (new_inner, new_utc, new_local) for platform-specific instantiation, and strftime/asctime logic is refactored to branch on Unix versus non-Unix implementations.

Changes

Cohort / File(s)	Summary
Time Module Refactoring `crates/vm/src/stdlib/time.rs`	Comprehensive platform-specific time handling: function signatures updated to accept `OptionalArg<Option<Either<f64, i64>>>` for gmtime/localtime/ctime; new helper functions (pyobj_to_time_t, struct_time_from_tm, etc.) for Unix and non-Unix paths; StructTimeData constructors refactored with new_inner/new_utc/new_local variants; asctime and strftime expanded with Unix-specific and fallback implementations; slot_new binding adjusted to tuple unpacking; PyUtf8StrRef replaced with PyStrRef in imports.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

fix StructTimeData::new #6330: Directly modifies StructTimeData construction by introducing new_inner/new_utc/new_local constructors and updates gmtime/localtime to use them.
impl {raise_,str}signal #6411: Modifies strftime logic with platform-specific year formatting handling, complementing the strftime restructuring in this change.
Fix time.strptime #6208: Alters struct_time and related function signatures in the same time module, modifying strptime return type handling.

Poem

🐰 Time flows swift through Unix and beyond,
New paths branch where platforms bond,
Helpers spring forth, fresh and fine,
Struct_time constructors realign,
Through all our clocks, our code shines bright!

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately reflects the main changes: updating test_time and implementing additional time module functionality with platform-specific support (Unix/non-Unix paths).
Docstring Coverage	✅ Passed	Docstring coverage is 81.48% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

- Replace chrono-based gmtime/localtime/mktime/strftime/asctime/ctime with direct libc calls (gmtime_r, localtime_r, mktime, strftime) on unix, keeping chrono as fallback for non-unix platforms - Add checked_tm_from_struct_time for struct_time field validation - Accept None in gmtime/localtime/ctime (OptionalArg<Option<>>) - Fix StructTimeData field order: tm_zone before tm_gmtoff - Fix PyStructTime slot_new to accept optional dict argument

coderabbitai

Actionable comments posted: 4

🤖 Fix all issues with AI agents

In `@crates/vm/src/stdlib/time.rs`:
- Line 580: The error message uses the wrong function name: update the call to
checked_tm_from_struct_time in the ctime implementation so the third argument is
"ctime" (not "asctime"), ensuring checked_tm_from_struct_time(...) produces
correct error text for the ctime function; look for the call site in the ctime
function and replace the literal function-name argument accordingly.
- Around line 670-675: The current match on t.naive_or_local(vm) in the strftime
path swallows struct_time conversion errors and returns the raw format string
(using fmt_lossy), which hides user errors; instead propagate the conversion
error so callers get a proper exception like CPython does. Replace the Err(_) =>
return Ok(vm.ctx.new_str(...)) branch in the t.naive_or_local(vm) match with
propagation of the underlying error (or create and return an appropriate VM
error/Value) so that invalid struct_time inputs to the strftime implementation
raise rather than silently fall back.
- Around line 515-520: The non-Unix branch for localtime currently passes a
hardcoded 0 for tm_isdst (see secs.naive_or_local and
StructTimeData::new_local), which incorrectly claims DST is off; change this to
derive the DST flag from the chrono datetime if possible (compute whether the
local datetime is in DST and pass that value into StructTimeData::new_local as
tm_isdst) or, if accurate DST detection is not feasible, pass -1 to indicate
unknown DST instead of 0 so non-Unix platforms don't falsely report "not DST".
- Around line 836-841: new_local currently calls
chrono::Local.from_local_datetime(&tm).unwrap(), which can panic for DST-gap
times; change it to handle the MappedLocalTime result like the non-Unix mktime
path by using .single().ok_or_else(...) (or otherwise map the none case to a
PyErr) and propagate the error (or change new_local to return PyResult<Self>) so
you don't unwrap; update the callsite or signature accordingly and then pass the
resultant offset_seconds and tz_abbr into Self::new_inner as before.

🧹 Nitpick comments (2)

crates/vm/src/stdlib/time.rs (2)
947-964: i64 → f64 round-trip in pyobj_to_time_t can lose precision.

For the Either::B(int) branch, the value is cast to f64 (line 958), range-checked, then cast to time_t (line 963). On 64-bit platforms where time_t is i64, the f64 round-trip silently loses precision for values with magnitude > 2^53. Consider handling the integer path directly:
Proposed fix
     pub(super) fn pyobj_to_time_t(
         value: Either<f64, i64>,
         vm: &VirtualMachine,
     ) -> PyResult<time_t> {
-        let secs = match value {
+        match value {
             Either::A(float) => {
                 if !float.is_finite() {
                     return Err(vm.new_value_error("Invalid value for timestamp"));
                 }
-                float.floor()
+                let secs = float.floor();
+                if secs < time_t::MIN as f64 || secs > time_t::MAX as f64 {
+                    return Err(
+                        vm.new_overflow_error("timestamp out of range for platform time_t"),
+                    );
+                }
+                Ok(secs as time_t)
             }
-            Either::B(int) => int as f64,
-        };
-        if secs < time_t::MIN as f64 || secs > time_t::MAX as f64 {
-            return Err(vm.new_overflow_error("timestamp out of range for platform time_t"));
+            Either::B(int) => {
+                let ts: time_t = int
+                    .try_into()
+                    .map_err(|_| vm.new_overflow_error("timestamp out of range for platform time_t"))?;
+                Ok(ts)
+            }
         }
-        Ok(secs as time_t)
     }
1019-1078: Duplicated libc::tm construction between unix_mktime and checked_tm_from_struct_time.

Both functions extract the same fields from StructTimeData into a libc::tm with nearly identical code (clone + try_into_value + error mapping). Consider extracting a shared helper that builds the base libc::tm, then let each caller apply its own adjustments (validation, sentinel values, zone handling). As per coding guidelines: "When branches differ only in a value but share common logic, extract the differing value first, then call the common logic once to avoid duplicate code."

github-actions

📦 Library Dependencies

The following Lib/ modules were modified. Here are their dependencies:

[ ] lib: cpython/Lib/test/support
[x] test: cpython/Lib/test/test_support.py (TODO: 3)
[ ] test: cpython/Lib/test/test_script_helper.py

dependencies:

support (native: _imp, _interpchannels, _opcode, _thread, collections.abc, concurrent.interpreters, concurrent.interpreters._crossinterp, email._header_value_parser, errno, importlib.machinery, importlib.util, logging.handlers, math, os.path, select, sys, test.support, test.support.import_helper, test.support.script_helper, test.test_tools, time)
- annotationlib (native: builtins, sys)
- collections (native: _weakref, itertools, sys)
- enum (native: builtins, sys)
- hashlib
- io (native: _io, _thread, errno, sys)
- logging (native: atexit, errno, logging.handlers, sys, time)
- opcode (native: _opcode, builtins)
- pathlib (native: errno, itertools, pathlib._os, sys)
- shutil (native: errno, sys)
- stat
- subprocess (native: builtins, errno, sys, time)
- sysconfig (native: os.path, sys)
- tempfile (native: _thread, errno, sys)
- test (native: main, _ast, _codecs, _ctypes, _datetime, _imp, _interpchannels, _interpreters, _io, _locale, _multibytecodec, _opcode, _operator, _overlapped, _pickle, _pyrepl._module_completer, _pyrepl.console, _pyrepl.input, _pyrepl.keymap, _pyrepl.reader, _pyrepl.readline, _pyrepl.simple_interact, _pyrepl.terminfo, _pyrepl.utils, _ssl, _string, _sysconfig, _testcapi, _testconsole, _testinternalcapi, _testlimitedcapi, _testsinglephase, _thread, _tkinter, _winapi, array, asyncio.events, asyncio.log, asyncio.proactor_events, asyncio.selector_events, asyncio.staggered, atexit, binascii, builtins, cmath, collections.abc, compression._common, compression.zstd, concurrent.futures, concurrent.futures._base, concurrent.futures.interpreter, concurrent.futures.process, concurrent.futures.thread, concurrent.interpreters, concurrent.interpreters._crossinterp, ctypes.macholib.dyld, ctypes.macholib.dylib, ctypes.macholib.framework, ctypes.util, ctypes.wintypes, dbm.dumb, dbm.ndbm, dbm.sqlite3, email._header_value_parser, email._policybase, email.base64mime, email.charset, email.contentmanager, email.errors, email.generator, email.header, email.headerregistry, email.iterators, email.message, email.mime.application, email.mime.audio, email.mime.base, email.mime.image, email.mime.message, email.mime.multipart, email.mime.nonmultipart, email.mime.text, email.parser, email.policy, email.utils, ensurepip._uninstall, errno, faulthandler, fcntl, gc, html.entities, html.parser, http.client, http.cookiejar, http.cookies, http.server, idlelib.idle_test, importlib._bootstrap, importlib._bootstrap_external, importlib.abc, importlib.machinery, importlib.metadata, importlib.readers, importlib.resources, importlib.resources._adapters, importlib.resources.abc, importlib.util, itertools, logging.config, logging.handlers, marshal, math, mmap, msvcrt, multiprocessing.connection, multiprocessing.dummy, multiprocessing.heap, multiprocessing.managers, multiprocessing.pool, multiprocessing.process, multiprocessing.queues, multiprocessing.util, not_a_module, os.path, package.submodule, parse, pyexpat, readline, select, setuptools, sqlite3.main, string.templatelib, sys, sysconfig.main, termios, test._code_definitions, test._crossinterp_definitions, test._test_multiprocessing, test.fork_wait, test.libregrtest, test.libregrtest.filter, test.libregrtest.main, test.libregrtest.result, test.libregrtest.results, test.libregrtest.utils, test.list_tests, test.picklecommon, test.pickletester, test.profilee, test.ssl_servers, test.string_tests, test.support, test.support.ast_helper, test.support.bytecode_helper, test.support.hashlib_helper, test.support.hypothesis_helper, test.support.i18n_helper, test.support.import_helper, test.support.logging_helper, test.support.numbers, test.support.os_helper, test.support.pty_helper, test.support.script_helper, test.support.socket_helper, test.support.testcase, test.support.threading_helper, test.support.venv, test.support.warnings_helper, test.test__interpreters, test.test_ast.snippets, test.test_ast.utils, test.test_asyncio, test.test_asyncio.utils, test.test_capi.test_getargs, test.test_contextlib, test.test_ctypes, test.test_ctypes._support, test.test_dbm, test.test_doctest, test.test_doctest.decorator_mod, test.test_email, test.test_httplib, test.test_httpservers, test.test_import, test.test_import.data.circular_imports.binding, test.test_import.data.circular_imports.binding2, test.test_import.data.circular_imports.import_cycle, test.test_import.data.circular_imports.subpkg2.parent, test.test_import.data.circular_imports.subpkg2.parent.child, test.test_importlib, test.test_importlib.extension.test_loader, test.test_importlib.metadata.fixtures, test.test_importlib.util, test.test_inspect, test.test_json, test.test_math, test.test_module.final_a, test.test_module.final_b, test.test_profile, test.test_py_compile, test.test_pydoc, test.test_pyrepl, test.test_sqlite3, test.test_string._support, test.test_tkinter.support, test.test_tkinter.widget_tests, test.test_tools, test.test_ttk_textonly, test.test_unittest, test.test_unittest.support, test.test_unittest.test_result, test.test_unittest.testmock, test.test_unittest.testmock.support, test.test_warnings.data, test.test_zoneinfo, test.test_zoneinfo._support, test.tracedmodules, test.typinganndata, test.typinganndata.ann_module, test_regrtest_b.util, time, tkinter.colorchooser, tkinter.commondialog, tkinter.messagebox, tkinter.simpledialog, unicodedata, unittest.case, unittest.mock, unittest.util, urllib.error, urllib.parse, urllib.request, urllib.response, urllib.robotparser, winreg, wsgiref.handlers, wsgiref.headers, wsgiref.simple_server, wsgiref.util, wsgiref.validate, xml.dom, xml.dom.minicompat, xml.dom.minidom, xml.etree, xml.etree.ElementTree, xml.parsers, xml.parsers.expat, xml.sax, xml.sax.expatreader, xml.sax.handler, xml.sax.saxutils, xml.sax.xmlreader, xmlrpc.client, xmlrpc.server, zipfile._path)
- types
- unittest (native: builtins, os.path, sys, time, unittest.util)
- venv (native: sys)
- ast, contextlib, dataclasses, dis, functools, getopt, importlib, inspect, os, py_compile, re, selectors, shlex, socket, string, textwrap, threading, warnings

dependent tests: (no tests depend on support)

[ ] test: cpython/Lib/test/test_time.py (TODO: 24)
[ ] test: cpython/Lib/test/test_strftime.py

dependencies:

dependent tests: (133 tests)

time: test_android test_asyncio test_calendar test_compileall test_concurrent_futures test_context test_dummy_thread test_dummy_threading test_email test_epoll test_file_eintr test_float test_fork1 test_ftplib test_functools test_http_cookiejar test_httpservers test_imaplib test_importlib test_inspect test_int test_io test_kqueue test_logging test_mailbox test_multiprocessing_main_handling test_os test_plistlib test_poll test_posix test_queue test_random test_resource test_sched test_selectors test_signal test_smtplib test_socket test_ssl test_strftime test_structseq test_subprocess test_sys test_syslog test_thread test_threading test_threadsignals test_time test_timeout test_urllibnet test_weakref test_xmlrpc test_zipfile test_zipfile64 test_zipimport
- email.generator: test_email
  - mailbox: test_genericalias
  - smtplib: test_smtpnet
- email.utils:
  - http.server: test_robotparser test_urllib2_localnet
- http.server:
  - wsgiref.simple_server: test_wsgiref
  - xmlrpc.server: test_docxmlrpc
- multiprocessing: test_concurrent_futures test_fcntl
- pydoc: test_enum
- queue: test_asyncio
- socketserver: test_socketserver
- subprocess: test_audit test_bz2 test_c_locale_coercion test_cmd_line test_cmd_line_script test_ctypes test_dtrace test_faulthandler test_gzip test_json test_msvcrt test_ntpath test_osx_env test_platform test_py_compile test_regrtest test_repl test_runpy test_script_helper test_select test_shutil test_site test_sqlite3 test_support test_sysconfig test_tempfile test_unittest test_urllib2 test_utf8_mode test_venv test_wait3 test_webbrowser
  - ctypes.util: test_ctypes
  - ensurepip: test_ensurepip
- tarfile: test_tarfile
- threading: test_asyncio test_code test_concurrent_futures test_contextlib test_decimal test_hashlib test_httplib test_importlib test_itertools test_largefile test_linecache test_opcache test_super test_termios test_threadedtempfile test_threading_local test_winreg test_zstd
  - sysconfig: test_pyexpat test_tools test_urllib2net
  - trace: test_trace
  - zipfile: test_pkgutil test_zipapp test_zipfile
- timeit: test_timeit
- urllib.request: test_pathlib test_urllib
- zipfile:
  - importlib.metadata: test_importlib test_zoneinfo
- zipimport: test_importlib

Legend:

[+] path exists in CPython
[x] up-to-date, [ ] outdated

update test_time and implement more time module

youknowone force-pushed the time branch 3 times, most recently from 933912a to f1e686e Compare February 11, 2026 04:21

youknowone changed the title ~~align time~~ Feb 11, 2026

youknowone commented Feb 11, 2026

View reviewed changes

youknowone and others added 2 commits February 11, 2026 16:54

Update test_time from v3.14.3

c4f2329

youknowone force-pushed the time branch from f1e686e to c4f2329 Compare February 11, 2026 07:58

youknowone marked this pull request as ready for review February 11, 2026 07:59

coderabbitai Bot reviewed Feb 11, 2026

View reviewed changes

youknowone deleted the time branch February 11, 2026 09:29

This was referenced Feb 13, 2026

Use CRT functions for time module on Windows #7090

Merged

Add gh-aw agentic workflow for upgrading Python libraries #7117

Merged

Update test_unicodedata from v3.14.2 and implement more #7114

Merged

coderabbitai Bot mentioned this pull request Feb 27, 2026

Disallow warnings patches #7249

Merged

youknowone added a commit to youknowone/RustPython that referenced this pull request Mar 22, 2026

Merge pull request RustPython#7073 from youknowone/time …

3f9b176

update test_time and implement more time module

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update test_time and implement more time module#7073