Walkthrough

Refactoring of the StructTimeData constructor in the time module. The single public new method is replaced with three specialized constructors: new_inner for direct construction with explicit parameters, new_utc for UTC times, and new_local for local times. Existing call sites in gmtime and localtime are updated to use the appropriate new constructors.

Changes

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Verify the removed new method is not called elsewhere in the codebase that isn't shown in this diff
• Confirm new_utc and new_local correctly delegate parameters to new_inner with appropriate defaults
• Validate that gmtoff and zone calculations are correct for UTC and local time variants

Poem

🐰 A constructor split in three,
New paths where one used to be,
UTC and local ways,
Time zones dancing through our days,
Cleaner code, we hop with glee!

Pre-merge checks and finishing touches

• 📝 Generate docstrings

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

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Reviewing files that changed from the base of the PR and between c826f9d and f14469b.

• crates/vm/src/stdlib/time.rs (4 hunks)

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.rs: Follow the default rustfmt code style by running cargo fmt to format Rust code
Always run clippy to lint Rust code (cargo clippy) before completing tasks and fix any warnings or lints introduced by changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass, pymodule, pyfunction, etc.) when implementing Python functionality in Rust

Files:

• crates/vm/src/stdlib/time.rs

• GitHub Check: Run snippets and cpython tests (macos-latest)
• GitHub Check: Run snippets and cpython tests on wasm-wasi
• GitHub Check: Check the WASM package and demo
• GitHub Check: Run snippets and cpython tests (ubuntu-latest)
• GitHub Check: Run rust tests (ubuntu-latest)
• GitHub Check: Run rust tests (windows-latest)
• GitHub Check: Check Rust code with clippy
• GitHub Check: Run rust tests (macos-latest)

crates/vm/src/stdlib/time.rs (4)

308-315: LGTM!

The refactoring to use new_utc for gmtime is correct. UTC always has isdst=0, gmtoff=0, and zone "UTC".

---

317-326: LGTM!

The call to new_local with isdst=-1 correctly represents an unknown DST state, as noted by the existing TODO comment.

---

503-523: LGTM!

The new_inner constructor is clean and correctly initializes all struct fields with the provided parameters.

---

530-537: Verify DST transition handling against CPython behavior before applying fixes.

The review identifies two concerns in new_local():

1. .unwrap() on from_local_datetime() may panic during DST gaps
2. Offset calculation may double-count DST

However, verification cannot proceed without repository access. Critically, Python's time.localtime() is a thin wrapper around the platform C library, which handles DST transitions, ambiguous times, and non-existent times according to platform-specific semantics. RustPython's chrono-based Rust implementation may intentionally or unintentionally diverge from this behavior.

Before applying the proposed diff:

• Confirm the actual code at lines 530–537
• Verify whether isdst == 1 is ever passed to new_local() (affects offset double-counting risk)
• Test DST transition behavior against CPython on the target platform to ensure compatibility
• Determine if errors should be propagated (returning PyResult) or handled by picking a canonical time (e.g., .earliest())

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.

• X
• Mastodon
• Reddit
• LinkedIn

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