fix: derive SEASON_PCT from date range instead of hardcoding half-season (#9) #36
No reviewers
Labels
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: cal/paper-dynasty-card-creation#36
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "ai/paper-dynasty-card-creation#9"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #9
What changed
SEASON_PCTwas hardcoded to81 / 162(~0.5) whileEND_DATEwas set to20050731(~65% through the season). Runningretrosheet_data.pydirectly — without thepd-cardsCLI, which overridesSEASON_PCTat runtime viard.SEASON_PCT = season_pct— would silently generate cards with half-season stat normalizations on a 65%-season dataset.Fix
START_DATEandEND_DATEdeclarations beforeSEASON_PCTso the derivation can reference themSEASON_END_DATE = 20051002(2005 MLB regular season end) as the denominator anchorSEASON_PCTis now derived:(END_DATE - START_DATE).days / (SEASON_END_DATE - START_DATE).days, capped at 1.0START_DATE=20050403,END_DATE=20050731),SEASON_PCTevaluates to~0.653instead of0.500Before: Changing
END_DATEsilently leftSEASON_PCTstale.After: Changing
END_DATEautomatically produces the correctSEASON_PCT.Files changed
retrosheet_data.py— config block only (lines 64–79)Other observations
retrosheet_data.py:1696-1699) — commented-out promo safety guard — is still present and unaddressed; tracked separately.LAST_WEEK_RATIOdead ternary) is also still present; tracked separately.AI Code Review
Files Reviewed
retrosheet_data.py(modified)Findings
Correctness
SEASON_PCTformula is mathematically correct. With defaultsSTART_DATE=20050403,END_DATE=20050731,SEASON_END_DATE=20051002: numerator = 119 days, denominator = 182 days → 0.654, consistent with the claimed ~0.653.datetimeis already imported at line 2 — no missing dependency.min(..., 1.0)cap correctly handles anyEND_DATE > SEASON_END_DATEedge case.START_DATEandEND_DATEwere already declared beforeSEASON_PCTin the original file (the diff confirms they were in place, just reordered within the block relative to the removed line). The derivation is correct regardless.SEASON_END_DATE == START_DATE, which is physically impossible in practice and acceptable in a hand-edited config file.Security
Style & Conventions
p_namekey innew_player_payload(CLAUDE.md notes the API expectsname, notp_name) is out of scope for this PR and should be tracked separately.Suggestions
SEASON_END_DATEis hardcoded to the 2005 regular season end. This is fine given the file is explicitly 2005-targeted throughout, but a comment noting it should be updated for other seasons would help future maintainers (non-blocking).Verdict: APPROVED
Clean fix for a real silent-failure bug. The derived formula is correct, the
datetimemodule is available, and the ruff-formatted f-strings are safe cosmetic changes. No blocking issues.Automated review by Claude PR Reviewer
AI Code Review
Files Reviewed
retrosheet_data.py(modified)Findings
Correctness
START_DATE=20050403,END_DATE=20050731,SEASON_END_DATE=20051002), the derivation yields119 / 182 ≈ 0.654, which matches the PR description's claimed~0.653. (Minor rounding difference — both are correct.)datetimeis already imported at the top of the file; thestrptimecalls in the newSEASON_PCTderivation are safe.min(..., 1.0)cap is correct defensive programming — prevents values slightly over 1.0 ifEND_DATEis set pastSEASON_END_DATE.START_DATE/END_DATEbeforeSEASON_PCTis required and correct; no forward-reference issues.Minor Observation — Inconsistency with existing helper
The file already has a
date_from_int(integer_date: int)helper that does the same conversion asdatetime.datetime.strptime(str(END_DATE), "%Y%m%d"). The PR usesstrptimeinstead, which is a small inconsistency. Not a bug — both produce the same result — but usingdate_from_intwould be more in keeping with the existing codebase style. This does not block merging.Security
Style & Conventions
SEASON_PCTas a manual config value (Percentage of season completed (162/162 for full season)). This is now mildly misleading since the value is derived automatically. Not a blocker, but a docs update would be worth doing in a follow-up.Suggestions
SEASON_PCTto reflect that it is now auto-derived fromSTART_DATE,END_DATE, andSEASON_END_DATE. Operators should be told to setEND_DATErather thanSEASON_PCTdirectly.date_from_inthelper could replace the twostrptimecalls in the derivation block for consistency, but this is purely a style preference.Verdict: APPROVED
The fix is correct, complete, and solves the stated problem cleanly. The silent stale-
SEASON_PCTbug is eliminated without any behavioral risk — the math produces the right value for existing defaults and will remain correct asEND_DATEis updated in future runs. The ruff formatting changes are noise-free. No blockers.Automated review by Claude PR Reviewer
Approved. Deriving SEASON_PCT from the actual date range rather than a hardcoded value is the correct approach — makes it accurate for any run date.
c568b6a024to63a30bd434