镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

fix(eva): handle DataFrame input in pred_autocorr - #2372

Open
Misty_Cirque (Cyber-Marty) wants to merge 2 commits into
microsoft:mainfrom
Cyber-Marty:fix/pred-autocorr-dataframe
Open

Misty_Cirque (Cyber-Marty) wants to merge 2 commits into
microsoft:mainfrom
Cyber-Marty:fix/pred-autocorr-dataframe

Conversation

@Cyber-Marty

Copy link
Copy Markdown

Description

Move the DataFrame warning before selecting its first column in pred_autocorr, so the warning can read the DataFrame's column names. Add a synthetic-data regression test that compares DataFrame input with its first Series column.

Motivation and Context

Fixes #2371. Previously, every DataFrame input raised AttributeError at pred.columns because pred had already become a Series. The documented Series path is unchanged.

How Has This Been Tested?

  • The new regression test fails on main with AttributeError: 'Series' object has no attribute 'columns' and passes with this change.
  • git diff --check passes.
  • Full data pipeline test (requires the pipeline dataset and was not run).

The regression test uses only synthetic predictions and no market data.

Types of changes

  • Fix bugs

@arhancanli

Copy link
Copy Markdown

The reorder is right: pred.columns was read after pred had already become a Series, so every DataFrame call raised. One problem with the regression test, though. It builds the DataFrame as {"score": first, "unused": -first} and asserts pred_autocorr(predictions) equals pred_autocorr(first). Autocorrelation here is a cross-sectional Pearson correlation, which does not change under a sign flip, so pred_autocorr(-first) is identical to pred_autocorr(first). I replayed the function body on the test data (pandas 2.3.3, copy of the loop, not qlib itself): both give [nan, -0.5, -0.5], and assert_series_equal passes between them. So the test would still pass if the code picked the second column, i.e. it checks "does not raise" but not "uses the first column".

A different second column fixes that, for example pd.Series([3, 1, 2, 1, 2, 3, 2, 3, 1], index=index), something that is not an affine function of the first one, and then assert against pred_autocorr(first) as now. Optionally also assert the warning is logged, since that is the line the PR actually moved.

@arhancanli

Copy link
Copy Markdown

Correction to my example column: I re-ran it and [3, 1, 2, 1, 2, 3, 2, 3, 1] also gives [nan, -0.5, -0.5], so it would not distinguish the columns either (with three instruments the cross-sectional correlations take very few values). [1, 2, 4, 1, 5, 2, 3, 1, 9] gives [nan, -0.693, -0.866] against [nan, -0.5, -0.5] for the original first column, so something like that (or more instruments) makes the assertion depend on which column is selected.

@Cyber-Marty

Copy link
Copy Markdown
Author

Thanks for pointing this out and correcting the example. I reproduced it with qlib's actual pred_autocorr on pandas 2.3.3: the original regression still passes when the implementation selects the second column. Both -first and the first suggested alternative produce [nan, -0.5, -0.5]; the corrected column produces [nan, -0.6933752452815366, -0.8660254037844387].

Updated in commit 44881c1:

  • Replaced the unused column with the corrected values, so the assertion now distinguishes first-column selection from second-column selection.
  • Added an assertion that the WARNING includes the DataFrame's column names. The test temporarily enables the qlib logger's propagation and restores its previous state afterward.
  • No additional production-code changes were needed.

Validation on Windows 11 / Python 3.12.14 / pandas 2.3.3 / NumPy 2.5.4:

  • The updated regression passes.
  • It fails with the pre-PR function (AttributeError), with second-column selection (different correlations), and with the warning removed. These checks replace the function only in memory; the source tree stays unchanged.
  • The targeted offline misc selection passes: 9 tests, with four existing dependency/untouched-code warnings.
  • Black 23.7.0, Flake8 4.0.1, and git diff --check pass.

No market data was downloaded, and the full data pipeline was not run. Thanks for helping make the regression test check its stated contract rather than just the absence of an exception.

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.

pred_autocorr raises AttributeError for DataFrame predictions

2 participants