Skip to content

Commit

Permalink
fixed minor ordering issue of combinedcost as visible through __repr__
Browse files Browse the repository at this point in the history
  • Loading branch information
enzbus committed Jan 9, 2024
1 parent 118e59b commit edda2e3
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 13 deletions.
14 changes: 12 additions & 2 deletions cvxportfolio/costs.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ def __add__(self, other):
by summing over costs.
"""
if isinstance(other, CombinedCosts):
return other + self
return other.__radd__(self)
return CombinedCosts([self, other], [1.0, 1.0])

def __rmul__(self, other):
Expand Down Expand Up @@ -132,17 +132,27 @@ def __init__(self, costs, multipliers):
self.do_convexity_check = True

def __add__(self, other):
"""Add other (combined) cost to self."""
"""Add self to other (combined) cost."""
if isinstance(other, CombinedCosts):
return CombinedCosts(self.costs + other.costs,
self.multipliers + other.multipliers)
return CombinedCosts(self.costs + [other], self.multipliers + [1.0])

def __radd__(self, other):
"""Add other (combined) cost to self."""
if isinstance(other, CombinedCosts):
return other + self # pragma: no cover
return CombinedCosts([other] + self.costs, [1.0] + self.multipliers)

def __mul__(self, other):
"""Multiply by constant."""
return CombinedCosts(self.costs,
[el * other for el in self.multipliers])

def __neg__(self):
"""Take negative of cost."""
return self * -1

def initialize_estimator_recursive(self, **kwargs):
"""Initialize iterating over constituent costs.
Expand Down
16 changes: 10 additions & 6 deletions cvxportfolio/hyperparameters.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,21 +129,25 @@ def __repr__(self):
expressions in general. It's not perfect but readable.
"""

# TODO this gives wrong string repr with nested expressions like
# ``-(cvx.Gamma() * -3 * (cvx.Gamma() - cvx.Gamma()))``
# internal algebra is correct, though

def _minus_repr(obj):
rawrepr = str(obj).lstrip()
if rawrepr[0] == '-':
return ' +' + rawrepr[1:].lstrip()
return ' + ' + rawrepr[1:].lstrip()
if rawrepr[0] == '+':
return ' -' + rawrepr[1:].lstrip() # pragma: no cover
return ' -' + rawrepr
return ' - ' + rawrepr[1:].lstrip() # pragma: no cover
return ' - ' + rawrepr

def _plus_repr(obj):
rawrepr = str(obj).lstrip()
if rawrepr[0] == '-':
return ' -' + rawrepr[1:].lstrip()
return ' - ' + rawrepr[1:].lstrip()
if rawrepr[0] == '+':
return ' +' + rawrepr[1:].lstrip()
return ' +' + str(obj)
return ' + ' + rawrepr[1:].lstrip()
return ' + ' + rawrepr

result = ''

Expand Down
10 changes: 5 additions & 5 deletions cvxportfolio/tests/test_hyperparameters.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,11 @@ def test_repr(self):
- cvx.Gamma() * cvx.StocksTransactionCost()

self.assertTrue(str(obj) ==
'-Gamma(current_value=1.0) * '
+ 'FullCovariance(Sigma=HistoricalFactorizedCovariance(kelly=True))'
+ ' + ReturnsForecast(r_hat=HistoricalMeanReturn(), decay=1.0)'
+ '-Gamma(current_value=1.0) * StocksTransactionCost(a=0.0,'
+ ' pershare_cost=0.005, b=1.0, exponent=1.5)')
'ReturnsForecast(r_hat=HistoricalMeanReturn(), decay=1.0)'
+ '- Gamma(current_value=1.0) * FullCovariance('
+ 'Sigma=HistoricalFactorizedCovariance(kelly=True))'
+ '- Gamma(current_value=1.0) * StocksTransactionCost('
+ 'a=0.0, pershare_cost=0.005, b=1.0, exponent=1.5)')

print(cvx.Gamma() * cvx.Gamma())
print(cvx.Gamma() - cvx.Gamma())
Expand Down

0 comments on commit edda2e3

Please sign in to comment.