Skip to content

fix: DSR moments, rolling attribution alignment, Hill lower tail, Polars 2 timestamps, empty frames, config errors - #90

Closed
quantum-like wants to merge 7 commits into
ml4t:mainfrom
quantum-like:review/fixes
Closed

quantum-like wants to merge 7 commits into
ml4t:mainfrom
quantum-like:review/fixes

Conversation

@quantum-like

Copy link
Copy Markdown
Contributor

Owning issue

Closes #89

Outcome

7 correctness fixes, one commit each, each with a focused test:

  • Return skewness and kurtosis divided 1/n central moments by the n−1 standard deviation, which understated both inputs to the deflated Sharpe ratio on short samples. They now use population moments (m3/m2^1.5, m4/m2^2). The Sharpe ratio itself is unchanged. Test: tightened tests/test_evaluation/test_moments.py::test_reference_scipy_skewness and ::test_reference_scipy_kurtosis to an exact SciPy match on 30 observations. (7237b26)
  • Rolling return attribution fit betas on uncleaned factor rows after NaN rows were dropped from returns, which shifted every lagged beta. Betas now use the same cleaned rows they are applied to. Test: tests/test_evaluation/test_factor_attribution.py::test_nan_row_matches_predropped_sample. (48787d0)
  • The lower Hill tail sorted absolute returns, so large gains entered the loss tail. It now uses losses only. Test: tests/test_evaluation/test_distribution.py::test_hill_lower_tail_ignores_right_tail_spikes. (1d72278)
  • Follow-up: with the lower-tail fix, tail="both" raised on all-positive samples. It now falls back to the upper tail when no loss tail exists. Test: tests/test_evaluation/test_distribution.py::test_hill_both_on_positive_sample_uses_upper_tail. (8779ad5)
  • String factor timestamps used cast(Date)/cast(Datetime), which Polars 2 rejects. They are now parsed with str.to_date/str.to_datetime. Test: existing TestUtf8Timestamps (both tests were failing before). (84a452c)
  • Empty frames crashed outlier filtering (Polars 2 refuses abs() on a Null-dtype column). They are now returned unchanged. Test: existing test_empty_dataframe (was failing before). (947459a)
  • Non-mapping YAML/JSON configs (empty, null, list) raised TypeError. They now raise the documented ValueError. Test: tests/test_config/test_io.py::test_non_mapping_documents_raise_value_error. (b3c5628)

Compatibility

  • No public compatibility impact
  • Compatibility impact is documented in the owning issue and migration guidance

Deflated Sharpe inputs (skewness, kurtosis) change numerically on short samples, and rolling attribution betas change when returns contain NaN rows. Note that the distribution-moments API (evaluation/distribution/moments.py) still uses unbiased sample moments; both conventions are documented.

Verification

  • Tests reproduce the prior failure or verify the requested behavior (each listed test fails on main and passes with its fix)
  • Ruff lint and format checks pass (ruff format --check is clean on the changed files and ruff check reports nothing new in them versus main, but I did not run the full pre-commit suite)
  • ty passes (not run on this branch)
  • Test suite passes: Default suite (not slow): 5261 passed, 52 failed before; 5268 passed, 49 failed after (128 skipped both times). The fixes cleared 3 failures.
  • Package build passes when applicable (not run)
  • Strict MkDocs build passes when documentation is affected (no documentation changes)
  • Ecosystem qualification passes (not run)

The 49 remaining failures are environment limits in my run, not regressions, and were present before: SHAP is not installed (SHAP metric, interaction and end-to-end example tests fail on import shap), and one portfolio test fails inside the installed ml4t-backtest because NYSE rejects session_start_time='17:00'.

Documentation and release

No documentation changes. The behavior changes above may be worth a line in the next patch-release notes.

Amir

cursoragent and others added 7 commits October 7, 2026 07:39
Fisher skewness and Pearson kurtosis were divided by the sample standard
deviation. Their numerators already use the 1/n central moments, so short
samples understated both inputs to the deflated Sharpe ratio.

Co-authored-by: quantum-like <quantum-like@users.noreply.github.com>
Rolling betas were estimated on the original factor frame after NaN rows
had already been dropped from the excess-return vector. The second
alignment kept a prefix and shifted every lagged beta.

Co-authored-by: quantum-like <quantum-like@users.noreply.github.com>
The lower-tail order statistics used absolute returns, so large gains
entered the loss tail and could dominate the tail index.

Co-authored-by: quantum-like <quantum-like@users.noreply.github.com>
FactorData cast string timestamps with cast(Date), which Polars 2 rejects.
Parse ISO dates and datetimes explicitly and reject values that do not parse.

Co-authored-by: quantum-like <quantum-like@users.noreply.github.com>
An empty factor column is inferred as Null, and Polars 2 refuses abs() on
that dtype. Return the frame unchanged when there are no rows.

Co-authored-by: quantum-like <quantum-like@users.noreply.github.com>
Empty, null, and list documents reached the model constructor and raised
TypeError. Raise ValueError, matching the documented config error type.

Co-authored-by: quantum-like <quantum-like@users.noreply.github.com>
Negated order statistics are the right loss tail, but a positive sample
has no strictly negative threshold. The both-tail path now uses the upper
tail in that case instead of failing the log transform.

Co-authored-by: quantum-like <quantum-like@users.noreply.github.com>
stefan-jansen added a commit that referenced this pull request Oct 14, 2026
Parse ISO factor timestamps explicitly for Polars 2, retain datetime precision on Polars 1, reject partially parsed values, and pass empty signal frames through outlier filtering.

Preserves the useful fixes from quantum-like's PR #90 and adds the reproduced datetime regression repair. AI assistance from the original Cursor Agent commits and Codex review, repair and tests is disclosed in the focused PR.

Co-authored-by: quantum-like <quantum-like@users.noreply.github.com>
stefan-jansen added a commit that referenced this pull request Oct 14, 2026
Use population second, third and fourth moments consistently for DSR skewness and kurtosis while retaining sample standard deviation for Sharpe. Cover short samples through the public raw-return DSR workflow.

Preserves quantum-like's original fix from PR #90. AI assistance from the original Cursor Agent commit and Codex review and tests is disclosed in the focused PR.

Co-authored-by: quantum-like <quantum-like@users.noreply.github.com>
stefan-jansen added a commit that referenced this pull request Oct 14, 2026
…101)

Estimate rolling betas on the cleaned factor and return rows used for attribution, preventing NaN row removal from shifting lagged contributions.

Preserves quantum-like's original fix from PR #90. AI assistance from the original Cursor Agent commit and Codex review and verification is disclosed in the focused PR.

Co-authored-by: quantum-like <quantum-like@users.noreply.github.com>
stefan-jansen added a commit that referenced this pull request Oct 15, 2026
Estimate the lower Hill tail from losses rather than absolute returns. Use the estimable tail for positive-only and negative-only samples in combined-tail mode.

Preserves quantum-like's original fixes from PR #90 and adds the reproduced negative-only regression repair. AI assistance from the original Cursor Agent commits and Codex review, repair and tests is disclosed in the focused PR.

Co-authored-by: quantum-like <quantum-like@users.noreply.github.com>
stefan-jansen added a commit that referenced this pull request Oct 15, 2026
Reject non-mapping JSON and YAML config documents before constructing the model, raising the documented ValueError instead of TypeError.

Preserves quantum-like's original fix from PR #90. AI assistance from the original Cursor Agent commit and Codex review and verification is disclosed in the focused PR.

Co-authored-by: quantum-like <quantum-like@users.noreply.github.com>
@stefan-jansen

Copy link
Copy Markdown
Contributor

The seven repairs from this bundled PR have landed through focused replacements:

Thanks @quantum-like (Amir) for the original repairs. Contributor credit is retained in the source commits, focused PR descriptions and squash commits. Each focused head passed all 39 applicable checks against its current main base; its reviewed tree was verified equal to the resulting squash-main tree. All reported fixes are now on main at dc9768c. Package publication is a separate pending step. Closing this bundled PR as superseded by the merged replacements.

AI assistance: the original Cursor Agent authorship is retained in the source history. Codex reproduced the workflows, split and reviewed the changes, added the ISO-datetime and negative-only-tail regressions/repairs, ran verification, and prepared this maintainer closure.

@quantum-like

Copy link
Copy Markdown
Contributor Author

Thanks for landing all of these, and for the extra ISO-datetime and negative-only-tail regressions. Much appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants