fix: Compute joint null mask before calling rolling corr/cov stats #18246
Conversation
agossard
requested review from
ritchie46,
c-peters,
alexander-beedie,
MarcoGorelli,
reswqa and
orlp
as code owners
agossard
changed the title
github-actions
bot
added
fix
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #18246 +/- ##
==========================================
+ Coverage 80.23% 80.24% +0.01%
==========================================
Files 1500 1500
Lines 198871 198897 +26
Branches 2837 2837
==========================================
+ Hits 159556 159604 +48
+ Misses 38788 38768 -20
+ Partials 527 525 -2 ☔ View full report in Codecov by Sentry. |
|
Doubt the hypothesis test failure is due to this PR. Maybe it's being addressed by #18245 @MarcoGorelli ? |
| pl.rolling_corr("a", "lag_a", window_size=10, min_periods=5, ddof=1).tail(1) | ||
| ).item() | ||
|
|
||
| assert val_1 == val_2 |
There was a problem hiding this comment.
Can you also test the actual value here?
| pl.rolling_cov("a", "lag_a", window_size=10, min_periods=5, ddof=1).tail(1) | ||
| ).item() | ||
|
|
||
| assert val_1 == val_2 |
There was a problem hiding this comment.
Can you also test the actual value here?
There was a problem hiding this comment.
I changed the test as suggested and pushed. However, I also spent some time trying to put together a hypothesis test that would cross check these corr and cov functions against numpy. I could not get it to pass, and have an example frame which yields correlation > 1.0 :-/
df = pl.DataFrame(
{
"a": [0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 1.0, 0.0],
"b": [101.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.000061, 0.0],
}
)
df_corr = df.select(
pl.rolling_corr("a", "b", window_size=7, min_periods=5, ddof=1)
)
I don't have time to push more on this right now (or even this week maybe). But I will log a separate issue.
Fixes #18217