Skip to content
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

Fix resampler chaining issue for compound returns in core.py #402

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Pranav-20186017
Copy link

Bug Fix: Resample Operation in quantstats/_plotting/core.py

Description

This PR fixes a small bug in quantstats/_plotting/core.py (lines 292–298), where resample operations were separated into two statements. That caused a DatetimeIndexResampler object to be stored in returns or benchmark, triggering the error:

Error: numpy operations are not valid with resample. Use .resample(...).sum() instead

What Changed

Before:

if resample:
    returns = returns.resample(resample)
    returns = returns.last() if compound else returns.sum(axis=0)
    if isinstance(benchmark, _pd.Series):
        benchmark = benchmark.resample(resample)
        benchmark = benchmark.last() if compound else benchmark.sum(axis=0)

The returns and benchmark were first assigned to a resampler object and then incorrectly processed. This resulted in an error when attempting NumPy operations on the resampler object.

Updated Implementation:

if resample:
    if compound:
        returns = returns.resample(resample).last()
        if isinstance(benchmark, _pd.Series):
            benchmark = benchmark.resample(resample).last()
    else:
        returns = returns.resample(resample).sum()
        if isinstance(benchmark, _pd.Series):
            benchmark = benchmark.resample(resample).sum()

The resample and aggregation calls are now correctly chained. This approach ensures that returns and benchmark remain valid DataFrame/Series objects after resampling.

Motivation for the Change

  • When compound = True, .last() is applied to capture the final value for each resampled period
  • When compound = False, .sum() is applied to aggregate the values within each resampled period
  • This change avoids assigning resampler objects and prevents runtime errors

Benefits

  • Prevents errors caused by attempting NumPy operations on resampler objects
  • Restores the correct resampling and aggregation logic for both compound and non-compound cases
  • Maintains compatibility with Pandas 2.x, which enforces stricter rules for resamplers
  • Adheres to common chaining practices like df.resample(...).sum()

Please review this PR and let me know if you have any feedback or suggestions! I am happy to make further improvements if needed.

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.

1 participant