fix(parallelogram): drop the daily lookback window by date, not row count - #1221
fix(parallelogram): drop the daily lookback window by date, not row count#1221SAY-5 wants to merge 1 commit into
Conversation
…ount Signed-off-by: Sai Asish Y <say.apm35@gmail.com>
Pyright Type CompletenessView the full Project (full
Other symbols referenced but not exported by
Symbols without documentation:
Patch (exported symbols added or changed by this PR): 0.0% fully typed (0 / 1)
Patch symbol details
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1221 +/- ##
=======================================
Coverage 91.27% 91.27%
=======================================
Files 91 91
Lines 5411 5412 +1
Branches 692 692
=======================================
+ Hits 4939 4940 +1
Misses 338 338
Partials 134 134
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@SAY-5 Welcome to the repo and thanks for putting this PR together so quickly. |
priyam0k
left a comment
There was a problem hiding this comment.
The solution is correct. Let maintainers have their final review.
Summary of Changes
With
approximation_grain="D",parallelogram_olfremoved the lookback window withiloc[366 * lookback_years + leap_day:]. When the lookback year itself is a leap year (a triangle starting 2017 with a 2016 lookback), 366 rows lands on 2016-12-31 rather than 2017-01-01, so the non-leap frame keeps a one-day 2016 origin the leap frame does not have, the positionaljoinmisaligns the two, andfit()fails withValueError: operands could not be broadcast together with shapes (1,1,3,1) (1,1,4,1). The lookback is now dropped by date (index >= first_date), which is independent of leap days and leaves the same origins in both frames. The existing daily-grain expectations are unchanged.Related GitHub Issue(s)
Fixes #1219
Additional Context for Reviewers
Added
test_daily_grain_after_leap_yearwith the repro from the issue.Checklist
uv run pytest) and documentation changes (uv run --directory docs jb build . --builder=custom --custom-builder=doctest)