Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 21 additions & 5 deletions quantstats/stats.py
Original file line number Diff line number Diff line change
Expand Up @@ -749,7 +749,8 @@ def rolling_volatility(
>>> print(rolling_vol)
"""
if prepare_returns:
returns = _utils._prepare_returns(returns, rolling_period)
# Note: this function has no rf parameter, so rf is never applied here.
returns = _utils._prepare_returns(returns, rolling_period, apply_rf=False)

# Calculate rolling standard deviation and annualize
return returns.rolling(rolling_period).std() * _np.sqrt(periods_per_year)
Expand Down Expand Up @@ -1487,8 +1488,9 @@ def gain_to_pain_ratio(returns, rf=0, resolution="D"):
Note:
See here for more info: https://archive.is/wip/2rwFW
"""
# Prepare returns and resample to specified frequency
returns = _utils._prepare_returns(returns, rf).resample(resolution).sum()
# Prepare returns and resample to specified frequency. rf is accepted for
# API compatibility but is not subtracted here (matches historical behavior).
returns = _utils._prepare_returns(returns, rf, apply_rf=False).resample(resolution).sum()

# Calculate absolute sum of negative returns (pain)
downside = abs(returns[returns < 0].sum())
Expand Down Expand Up @@ -2555,24 +2557,38 @@ def kelly_criterion(returns, prepare_returns=True):
Calculates the recommended maximum amount of capital that
should be allocated to the given strategy, based on the
Kelly Criterion (http://en.wikipedia.org/wiki/Kelly_criterion)

The growth-optimal fraction for a two-outcome return series is
f* = win_prob / |avg_loss| - lose_prob / avg_win, which factors to
(win_prob - lose_prob / win_loss_ratio) / |avg_loss|. The final
division by the average-loss magnitude is required to convert the
(scale-invariant) win/loss ratio into an actual capital fraction -
without it, the result is off by a factor of |avg_loss| and does not
change when the return series is scaled.
"""
if prepare_returns:
returns = _utils._prepare_returns(returns)
win_loss_ratio = payoff_ratio(returns)
win_prob = win_rate(returns)
lose_prob = 1 - win_prob
avg_loss_val = avg_loss(returns)

# Handle both Series (DataFrame input) and scalar (Series input) cases
if isinstance(win_loss_ratio, _pd.Series):
# DataFrame input - element-wise operations with zero/nan protection
# Replace 0 and NaN values with NaN to avoid division issues
win_loss_ratio_safe = win_loss_ratio.replace(0, _np.nan)
return ((win_loss_ratio_safe * win_prob) - lose_prob) / win_loss_ratio_safe
avg_loss_safe = abs(avg_loss_val).replace(0, _np.nan)
kelly_fraction = ((win_loss_ratio_safe * win_prob) - lose_prob) / win_loss_ratio_safe
return kelly_fraction / avg_loss_safe
else:
# Series input - scalar operations
if win_loss_ratio == 0 or _pd.isna(win_loss_ratio):
return _np.nan
return ((win_loss_ratio * win_prob) - lose_prob) / win_loss_ratio
if avg_loss_val == 0 or _pd.isna(avg_loss_val):
return _np.nan
kelly_fraction = ((win_loss_ratio * win_prob) - lose_prob) / win_loss_ratio
return kelly_fraction / abs(avg_loss_val)


# ==== VS. BENCHMARK ====
Expand Down
57 changes: 29 additions & 28 deletions quantstats/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import numpy as _np
from ._compat import safe_yfinance_download
from ._compat import safe_concat, safe_resample
import inspect
import threading

# Type alias for return data
Expand Down Expand Up @@ -107,7 +106,7 @@ def validate_input(data, allow_empty=False):
_cache_lock = threading.Lock()


def _generate_cache_key(data, rf, nperiods):
def _generate_cache_key(data, rf, nperiods, apply_rf=True):
"""
Generate a cache key for the _prepare_returns function

Expand All @@ -119,6 +118,8 @@ def _generate_cache_key(data, rf, nperiods):
Risk-free rate parameter
nperiods : int
Number of periods parameter
apply_rf : bool
Whether the risk-free rate is applied (excess returns are computed)

Returns
-------
Expand All @@ -134,8 +135,10 @@ def _generate_cache_key(data, rf, nperiods):
else:
data_hash = hash(str(data))

# Include parameters in the key
key = f"{data_hash}_{rf}_{nperiods}"
# Include parameters in the key (apply_rf must be part of the key since
# two calls with identical data/rf/nperiods can still want different
# treatment of rf depending on the caller)
key = f"{data_hash}_{rf}_{nperiods}_{apply_rf}"
return key
except (ValueError, TypeError, AttributeError, MemoryError):
# If hashing fails, return None to skip caching
Expand Down Expand Up @@ -580,7 +583,7 @@ def _prepare_prices(data, base=1.0):
return data


def _prepare_returns(data, rf=0.0, nperiods=None):
def _prepare_returns(data, rf=0.0, nperiods=None, apply_rf=True):
"""
Convert price data into returns and perform cleanup

Expand All @@ -592,22 +595,27 @@ def _prepare_returns(data, rf=0.0, nperiods=None):
Risk-free rate
nperiods : int, optional
Number of periods for risk-free rate conversion
apply_rf : bool, default True
Whether `rf` should be subtracted to compute excess returns here.
Callers that need `rf` for something other than excess returns
(e.g. because they annualize it separately, or because a caller
further up the stack already applied it) should pass False
explicitly rather than relying on this function to infer intent
from who is calling it.

Returns
-------
pd.Series or pd.DataFrame
Cleaned returns data
"""
# Try to get from cache first
cache_key = _generate_cache_key(data, rf, nperiods)
cache_key = _generate_cache_key(data, rf, nperiods, apply_rf)
if cache_key:
with _cache_lock:
if cache_key in _PREPARE_RETURNS_CACHE:
return _PREPARE_RETURNS_CACHE[cache_key].copy()

data = data.copy()
# Get calling function name for conditional processing
function = inspect.stack()[1][3]

# Process DataFrame columns
if isinstance(data, _pd.DataFrame):
Expand All @@ -628,24 +636,16 @@ def _prepare_returns(data, rf=0.0, nperiods=None):
if isinstance(data, (_pd.DataFrame, _pd.Series)):
data = data.fillna(0).replace([_np.inf, -_np.inf], float("NaN"))

# Functions that don't need excess returns calculation
unnecessary_function_calls = [
"_prepare_benchmark",
"cagr",
"gain_to_pain_ratio",
"rolling_volatility",
]

# Calculate excess returns if rf > 0 and function needs it
if function not in unnecessary_function_calls:
if rf > 0:
result = to_excess_returns(data, rf, nperiods)
# Cache the result
if cache_key:
_clear_cache_if_full()
with _cache_lock:
_PREPARE_RETURNS_CACHE[cache_key] = result.copy()
return result
# Calculate excess returns if the caller wants rf applied and rf is non-zero.
# (Negative rf is valid too, e.g. negative-yielding cash - only rf == 0 is a no-op.)
if apply_rf and rf != 0:
result = to_excess_returns(data, rf, nperiods)
# Cache the result
if cache_key:
_clear_cache_if_full()
with _cache_lock:
_PREPARE_RETURNS_CACHE[cache_key] = result.copy()
return result

# Normalize timezone information for consistency
# Convert to UTC if timezone-aware, then make naive
Expand Down Expand Up @@ -737,9 +737,10 @@ def _prepare_benchmark(benchmark=None, period="max", rf=0.0, prepare_returns=Tru
benchmark = benchmark.tz_convert('UTC').tz_localize(None)
# If already timezone-naive, no action needed

# Prepare returns or return raw data
# Prepare returns or return raw data. rf is not applied here (excess
# returns for a benchmark are computed by the caller where needed).
if prepare_returns:
return _prepare_returns(benchmark.dropna(), rf=rf)
return _prepare_returns(benchmark.dropna(), rf=rf, apply_rf=False)
return benchmark.dropna()


Expand Down
43 changes: 43 additions & 0 deletions tests/test_stats.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,49 @@ def test_cagr(self, sample_returns):
result = stats.cagr(sample_returns)
assert np.isfinite(result)

def test_cagr_with_rf(self, sample_returns):
"""rf should actually be subtracted (excess-return CAGR), not ignored.

Regression test for issue #537: `_prepare_returns` used to route rf
handling by inspecting the calling function's name via
`inspect.stack()`, and hard-excluded "cagr" from ever applying rf,
so `cagr(returns, rf=X)` was identical for every X.
"""
result_no_rf = stats.cagr(sample_returns, rf=0.0)
result_with_rf = stats.cagr(sample_returns, rf=0.02)
assert result_with_rf != result_no_rf
# A positive rf should reduce the excess-return CAGR.
assert result_with_rf < result_no_rf

def test_kelly_criterion(self):
"""kelly_criterion must divide by the average-loss magnitude.

Regression test for issue #537: the formula
``((win_loss_ratio * win_prob) - lose_prob) / win_loss_ratio``
simplifies to a function of the win/loss *ratio* only, so it is
scale-invariant and off by a factor of |avg_loss| versus the
textbook growth-optimal Kelly fraction
``f* = win_prob / |avg_loss| - lose_prob / avg_win``.

Uses a deterministic two-outcome series (60 wins of +2%, 40 losses
of -1%) so the closed-form Kelly fraction is exactly checkable:
win_loss_ratio = 2, win_prob = 0.6, lose_prob = 0.4, so
f* = ((2 * 0.6) - 0.4) / 2 / 0.01 = 40.0.
"""
dates = pd.date_range("2020-01-01", periods=100, freq="D")
values = [0.02] * 60 + [-0.01] * 40
returns = pd.Series(values, index=dates)

result = stats.kelly_criterion(returns)
np.testing.assert_almost_equal(result, 40.0, decimal=6)

# Scale-dependence: halving/scaling the return series should scale
# the Kelly fraction inversely (it did not, before the fix).
half = stats.kelly_criterion(returns * 0.5)
tenth = stats.kelly_criterion(returns * 0.1)
np.testing.assert_almost_equal(half, result * 2, decimal=6)
np.testing.assert_almost_equal(tenth, result * 10, decimal=6)


class TestBenchmarkComparison:
"""Test benchmark comparison functions."""
Expand Down