-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enhancement and bugfix in report plots #1413
base: main
Are you sure you want to change the base?
Conversation
@you-n-g would you review please? |
# Long-Average | ||
t_df["long-average"] = t_df["Group1"] - pred_label.groupby(level="datetime")["label"].mean() | ||
# Long-benchmark | ||
benchmark = kwargs.get("benchmark", "average") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be better to have the benchmark as an explicit input parameter, and we can also add type hints.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed as suggested.
benchmark = pred_label.groupby(level="datetime")["label"].mean() | ||
elif isinstance(benchmark, pd.Series): | ||
benchmark_name = benchmark.name if bool(benchmark.name) else "benchmark" | ||
benchmark = benchmark.reindex(t_df.index) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a high possibility that the user-provided benchmark may not align with t_df.index. Perhaps we can add some common warnings in this case if the DataFrame doesn't have warning messages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it might not be necessary as it should be a common case especially for a global portfolio. Think about using CSI300 to benchmarking a portfolio which invests both A and H shares, for those mainland holidays which are not HK holidays.
1b6d12b
to
d021e5c
Compare
any further comments? @Fivele-Li |
Description
len(x) // N * i : len(x) // N * (i + 1)
misseslen(x) % N
names at end.Motivation and Context
How Has This Been Tested?
Generated plot locally.
Screenshots of Test Results (if appropriate):
Types of changes