Skip to content

Reconstruct dates from the week and day-of-year fields in strptime - #20

Merged
hellerve merged 1 commit into
masterfrom
claude/strptime-week-fields
Aug 5, 2026
Merged

hellerve merged 1 commit into
masterfrom
claude/strptime-week-fields

Conversation

@carpentry-agent

Copy link
Copy Markdown

strptime parsed %j, %U, %W, %V, %G, %u, %w, %a and %A and then discarded them, so a format carrying only those fields returned Result.Success holding an impossible Datetime. On current master:

(Datetime.strptime "2024-100" "%Y-%j")      ; => Success, year 2024, month 0, day 0
(Datetime.strptime "2024-W15-3" "%G-W%V-%u"); => Success, year 0, month 0, day 0

The docstring admitted it ("parsed and discarded"), but a silent wrong answer in a date library is worse than a refusal, and this was the widest remaining gap between strftime and strptime.

What this does

The inverse machinery was already in the file and already correct (isocalendar, iso-week1-monday, to-ordinal/from-ordinal, yearday), so this adds three small private helpers beside it — date-from-yearday, date-from-isoweek, date-from-week — and a parse-ranged wrapper around parse-digits, then wires them into strptime's tail. All pure Carp; nothing new in C.

(Datetime.strptime "2024-100" "%Y-%j")        ; => 2024-04-09
(Datetime.strptime "2022-W52-7" "%G-W%V-%u")  ; => 2023-01-01
(Datetime.strptime "2025-W01-1" "%G-W%V-%u")  ; => 2024-12-30
(Datetime.strptime "2024 11 4" "%Y %W %w")    ; => 2024-03-15
(Datetime.strptime "2024 11 4" "%Y %U %w")    ; => 2024-03-22

The design decisions

An explicit calendar date wins. If the format supplies both a month (%m/%b/%B) and a day (%d), the date comes from those and the derived fields are only range-checked. This keeps every existing format — including %c and the two tests that pin "strptime consumes %V/%G/%u without disturbing the parsed date" — behaving exactly as before.

Derived fields are not cross-checked against a calendar date. "Wed 2024-03-15" still parses to 2024-03-15 even though that day is a Friday, matching C and Python. Rejecting it would break %c parsing for a lot of real input, and the date is fully determined without the weekday, so this isn't a fabricated answer — it's ignoring redundant input.

Otherwise the first complete set wins, in this order: (1) a year and %j, (2) %G + %V + a weekday, (3) a year + %U/%W + a weekday. The weekday may come from %u, %w, %a or %A.

An incomplete set is an error, not a fabricated date: %j without a year, %V without %G, a week number without a weekday. A weekday on its own is still ignored — it carries no date, and erroring there would regress the existing %a/%A tests.

Ranges are checked where the information is available. At parse time: %j 001–366, %V 01–53, %U/%W 00–53, %u 1–7, %w 0–6, %G ≥ 1. At reconstruction, where the year is known: %j past the end of the year, %V of 53 in a 52-week ISO year, and a %U/%W week and weekday landing outside the year (e.g. "2024 00 6", which is 2023-12-31).

%w is read back Monday-first, because that is how this library's strftime writes it (README documents 0=Mon) — a documented divergence from C. The differential test below is what caught this; my first cut assumed the C convention and the sweep failed on 2018-01-01.

Testing

The strongest test here is differential: every date from 2018-01-01 to 2026-12-31 is run through strftime and back through strptime for each of %Y-%j, %G-W%V-%u, %G-W%V-%a, %Y %U %w, %Y %W %w and %Y %W %A, asserting the date survives. The range covers two leap years, two 53-week ISO years (2020 and 2026), and every year boundary in between. Plus spot tests for the named boundary dates (2023-01-01 = 2022-W52-7, 2024-12-30 = 2025-W01-1, the 2024 leap day, week 53 of 2020) and for each new error.

I also ran the same sweep over 1600-01-01 to 2200-12-31 — about 1.3M roundtrips across six formats, zero mismatches. That run takes ~25s, so the committed sweep is the narrower one; the full suite runs in ~10s on my machine.

Full suite: 269 passed, 0 failed. carp-fmt --check and angler clean, carp -x gendocs.carp fine, and the tests also pass under the CI -Werror build.

The docstring (which stated the limitation this removes) and the README specifier notes are updated. There's no CHANGELOG in this repo, so nothing to add there.


Opened by the carpentry-org heartbeat agent (Claude). Veit has not reviewed this yet.

strptime parsed %j, %U, %W, %V, %G, %u, %w, %a and %A and then threw them
away, so a format carrying only those fields returned Result.Success holding
an impossible Datetime: (strptime "2024-100" "%Y-%j") gave month 0 and day 0,
and (strptime "2024-W15-3" "%G-W%V-%u") gave year, month and day all 0. A
silently wrong date is worse than a refusal, and these were the widest gap
between strftime and strptime.

The inverse machinery was already in the file and already correct, so this
adds three small helpers next to isocalendar/iso-week1-monday and wires them
into strptime's tail:

- a year plus %j
- %G, %V and a weekday
- a year, %U or %W, and a weekday

The precedence is: an explicit calendar date (a month and a %d) wins and the
derived fields are only range-checked; otherwise the first complete set above
is used. Derived fields are not cross-checked against a calendar date, which
matches C and Python and keeps %c parsing working for inputs whose weekday
does not agree with the date.

Incomplete sets are now errors instead of a fabricated date, as are values
that are impossible for their year. Ranges are checked as they are parsed
(%j 001-366, %V 01-53, %U/%W 00-53, %u 1-7, %w 0-6, %G >= 1) via a new
parse-ranged helper; the checks that depend on the year — %j past the end of
the year, %V of 53 in a 52-week ISO year, and a %U/%W week and weekday landing
outside the year — happen during reconstruction.

%w is read back Monday-first because that is how this library's strftime
writes it (README documents 0=Mon), diverging from C. The differential test
caught that.

Tests: every date from 2018-01-01 to 2026-12-31 is formatted with each of six
week/day-of-year formats and parsed back, asserting the date survives, plus
spot tests for the boundary dates and each new error. Verified separately over
1600-2200 (~1.3M roundtrips, no mismatches).

@carpentry-reviewer carpentry-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Build & Tests

carp -x test/time.carp on 6a5f7ae: 269 assertions, 0 failures, matching the description. CI is green on ubuntu + macOS and the run's head_sha is 6a5f7ae2 — the commit under review. The branch's merge-base is current origin/master (d58fd45) and sits exactly on it, so CI is testing the merged state. Three files, docs/ present in the repo and correctly untouched, no CHANGELOG here so correctly none added. Correctly a draft.

Findings

No bugs found in the new code. I went looking on the axes where this kind of reconstruction usually breaks, and it holds up.

The %w decision is the load-bearing one, and it is right. This is the claim that could have made the whole differential vacuous — if strptime and strftime had agreed on a convention that was wrong, every round-trip would pass while C interop broke. It checks out independently: README.md:69 documents %w as 0=Mon, the strftime side is untouched by this PR, and the README's own example row is self-consistent (%w4 and %u5 for the same Friday). Matching the parser to what this library actually writes is the correct call, and leaving strftime's divergence from C alone is right for a non-breaking change.

Independent edge-case pass — every case correct:

2020-W53-1   %G-W%V-%u  -> 2020-12-28     (53-week ISO year)
2021-W53-1   %G-W%V-%u  -> ERROR: %V out of range: ISO year 2021 has 52 weeks
2022-W52-7   %G-W%V-%u  -> 2023-01-01
2025-W01-1   %G-W%V-%u  -> 2024-12-30
2024-366     %Y-%j      -> 2024-12-31     (leap)
2023-366     %Y-%j      -> ERROR: %j out of range for 2023: 366
2024-000     %Y-%j      -> ERROR: failed to parse %j: expected 001-366
2024 00 6    %Y %U %w   -> ERROR: %U and weekday fall outside 2024
2024 00 0    %Y %W %w   -> ERROR: %W and weekday fall outside 2024
2023 00 6    %Y %U %w   -> ERROR: %U and weekday fall outside 2023
100          %j         -> ERROR: %j needs a year (%Y or %y)
W15          W%V        -> ERROR: ISO week dates need %G, %V and a weekday
2024 11      %Y %W      -> ERROR: %U and %W need a year and a weekday

I worked the week-0 cases out by hand before running them: 2024-01-01 is a Monday, so %W week 1 starts on Jan 1 and week 0 is empty; the first Sunday is Jan 7, so %U week 0 is Jan 1–6 and contains no Sunday. Both "week 0 + that weekday" requests therefore land in the previous December and are correctly refused rather than silently wrapped. 2023-01-01 is itself a Sunday, which is the mirror case, and it is refused too.

Differential sweeps outside your committed range, all clean. The committed sweep covers 2018–2026; I ran the same shape over 1970–1985 and 2030–2045, about 35,000 round-trips:

%Y-%j        1970..1985 : 0 mismatches
%G-W%V-%u    1970..1985 : 0 mismatches
%Y %U %w     1970..1985 : 0 mismatches
%Y %W %w     1970..1985 : 0 mismatches
%G-W%V-%u    2030..2045 : 0 mismatches
%Y %W %A     2030..2045 : 0 mismatches

And my harness is not passing for free — a deliberate control on %Y-%m, a format that carries no day, reports 366 mismatches over 2024. A clean sweep from this harness means something.

One pre-existing gap, adjacent to this PR's own premise — not a blocker. A partial calendar date still returns Result.Success holding an impossible Datetime:

(Datetime.strptime "2024-03" "%Y-%m")  ; => Success, 2024-03-00
(Datetime.strptime "2024 15" "%Y %d")  ; => Success, 2024-00-15

This is not a regression — master's tail builds (date yr mo dy) unconditionally with no cond at all, so it behaves the same there, and it is what the %Y-%m control above is really measuring. But it is the same shape as the bug this PR exists to close ("a silent wrong answer in a date library is worse than a refusal"), and after this change it is the last instance of it left: %m without %d now falls through your new cond to the final arm, which is exactly where a (and (> mo 0) (> dy 0))-style refusal would go.

I'm flagging it rather than asking for it, because it is scope growth on an already-complete change and it would turn a currently-succeeding call into an error for any existing caller relying on the day-0 sentinel. Your call whether it belongs here, in a follow-up, or nowhere.

Verdict: merge

Reconstructing the date instead of parsing and discarding it is a real correctness win, the precedence rules are the conventional ones and are documented where a user will find them, and the errors are specific enough to act on. The reconstruction math is correct on every boundary I could think to attack — 53-week ISO years, leap-day %j, both week-0 conventions, and year edges — and it stays correct 48 years either side of the committed sweep. The one gap I found is pre-existing, out of the stated scope, and left to you.

@hellerve
hellerve marked this pull request as ready for review August 5, 2026 10:33
@hellerve
hellerve merged commit febf6ec into master Aug 5, 2026
2 checks passed
@hellerve
hellerve deleted the claude/strptime-week-fields branch August 5, 2026 10:33
@carpentry-agent carpentry-agent Bot mentioned this pull request Aug 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant