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

Refactor SetHoldings to return a List<OrderTicket> #8550

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
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
44 changes: 28 additions & 16 deletions Algorithm/QCAlgorithm.Trading.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1326,23 +1326,27 @@ public void SetMaximumOrders(int max)
/// <param name="liquidateExistingHoldings">True will liquidate existing holdings</param>
/// <param name="tag">Tag the order with a short string.</param>
/// <param name="orderProperties">The order properties to use. Defaults to <see cref="DefaultOrderProperties"/></param>
/// <returns>A list of order tickets.</returns>
/// <seealso cref="MarketOrder(QuantConnect.Symbol, decimal, bool, string, IOrderProperties)"/>
[DocumentationAttribute(TradingAndOrders)]
public void SetHoldings(List<PortfolioTarget> targets, bool liquidateExistingHoldings = false, string tag = null, IOrderProperties orderProperties = null)
public List<OrderTicket> SetHoldings(List<PortfolioTarget> targets, bool liquidateExistingHoldings = false, string tag = null, IOrderProperties orderProperties = null)
{
var orderTickets = new List<OrderTicket>();
//If they triggered a liquidate
if (liquidateExistingHoldings)
{
Liquidate(GetSymbolsToLiquidate(targets.Select(t => t.Symbol)), tag: tag, orderProperties: orderProperties);
orderTickets = Liquidate(GetSymbolsToLiquidate(targets.Select(t => t.Symbol)), tag: tag, orderProperties: orderProperties);
}

foreach (var portfolioTarget in targets
// we need to create targets with quantities for OrderTargetsByMarginImpact
.Select(target => new PortfolioTarget(target.Symbol, CalculateOrderQuantity(target.Symbol, target.Quantity)))
.OrderTargetsByMarginImpact(this, targetIsDelta: true))
{
SetHoldingsImpl(portfolioTarget.Symbol, portfolioTarget.Quantity, false, tag, orderProperties);
var tickets = SetHoldingsImpl(portfolioTarget.Symbol, portfolioTarget.Quantity, false, tag, orderProperties);
orderTickets.AddRange(tickets);
}
return orderTickets;
}

/// <summary>
Expand All @@ -1353,11 +1357,12 @@ public void SetHoldings(List<PortfolioTarget> targets, bool liquidateExistingHol
/// <param name="liquidateExistingHoldings">liquidate existing holdings if necessary to hold this stock</param>
/// <param name="tag">Tag the order with a short string.</param>
/// <param name="orderProperties">The order properties to use. Defaults to <see cref="DefaultOrderProperties"/></param>
/// <returns>A list of order tickets.</returns>
/// <seealso cref="MarketOrder(QuantConnect.Symbol, decimal, bool, string, IOrderProperties)"/>
[DocumentationAttribute(TradingAndOrders)]
public void SetHoldings(Symbol symbol, double percentage, bool liquidateExistingHoldings = false, string tag = null, IOrderProperties orderProperties = null)
public List<OrderTicket> SetHoldings(Symbol symbol, double percentage, bool liquidateExistingHoldings = false, string tag = null, IOrderProperties orderProperties = null)
{
SetHoldings(symbol, percentage.SafeDecimalCast(), liquidateExistingHoldings, tag, orderProperties);
return SetHoldings(symbol, percentage.SafeDecimalCast(), liquidateExistingHoldings, tag, orderProperties);
}

/// <summary>
Expand All @@ -1368,11 +1373,12 @@ public void SetHoldings(Symbol symbol, double percentage, bool liquidateExisting
/// <param name="liquidateExistingHoldings">bool liquidate existing holdings if necessary to hold this stock</param>
/// <param name="tag">Tag the order with a short string.</param>
/// <param name="orderProperties">The order properties to use. Defaults to <see cref="DefaultOrderProperties"/></param>
/// <returns>A list of order tickets.</returns>
/// <seealso cref="MarketOrder(QuantConnect.Symbol, decimal, bool, string, IOrderProperties)"/>
[DocumentationAttribute(TradingAndOrders)]
public void SetHoldings(Symbol symbol, float percentage, bool liquidateExistingHoldings = false, string tag = null, IOrderProperties orderProperties = null)
public List<OrderTicket> SetHoldings(Symbol symbol, float percentage, bool liquidateExistingHoldings = false, string tag = null, IOrderProperties orderProperties = null)
{
SetHoldings(symbol, (decimal)percentage, liquidateExistingHoldings, tag, orderProperties);
return SetHoldings(symbol, (decimal)percentage, liquidateExistingHoldings, tag, orderProperties);
}

/// <summary>
Expand All @@ -1383,11 +1389,12 @@ public void SetHoldings(Symbol symbol, float percentage, bool liquidateExistingH
/// <param name="liquidateExistingHoldings">bool liquidate existing holdings if necessary to hold this stock</param>
/// <param name="tag">Tag the order with a short string.</param>
/// <param name="orderProperties">The order properties to use. Defaults to <see cref="DefaultOrderProperties"/></param>
/// <returns>A list of order tickets.</returns>
/// <seealso cref="MarketOrder(QuantConnect.Symbol, decimal, bool, string, IOrderProperties)"/>
[DocumentationAttribute(TradingAndOrders)]
public void SetHoldings(Symbol symbol, int percentage, bool liquidateExistingHoldings = false, string tag = null, IOrderProperties orderProperties = null)
public List<OrderTicket> SetHoldings(Symbol symbol, int percentage, bool liquidateExistingHoldings = false, string tag = null, IOrderProperties orderProperties = null)
{
SetHoldings(symbol, (decimal)percentage, liquidateExistingHoldings, tag, orderProperties);
return SetHoldings(symbol, (decimal)percentage, liquidateExistingHoldings, tag, orderProperties);
}

/// <summary>
Expand All @@ -1401,22 +1408,24 @@ public void SetHoldings(Symbol symbol, int percentage, bool liquidateExistingHol
/// <param name="liquidateExistingHoldings">bool flag to clean all existing holdings before setting new faction.</param>
/// <param name="tag">Tag the order with a short string.</param>
/// <param name="orderProperties">The order properties to use. Defaults to <see cref="DefaultOrderProperties"/></param>
/// <returns>A list of order tickets.</returns>
/// <seealso cref="MarketOrder(QuantConnect.Symbol, decimal, bool, string, IOrderProperties)"/>
[DocumentationAttribute(TradingAndOrders)]
public void SetHoldings(Symbol symbol, decimal percentage, bool liquidateExistingHoldings = false, string tag = null, IOrderProperties orderProperties = null)
public List<OrderTicket> SetHoldings(Symbol symbol, decimal percentage, bool liquidateExistingHoldings = false, string tag = null, IOrderProperties orderProperties = null)
{
SetHoldingsImpl(symbol, CalculateOrderQuantity(symbol, percentage), liquidateExistingHoldings, tag, orderProperties);
return SetHoldingsImpl(symbol, CalculateOrderQuantity(symbol, percentage), liquidateExistingHoldings, tag, orderProperties);
}

/// <summary>
/// Set holdings implementation, which uses order quantities (delta) not percentage nor target final quantity
/// </summary>
private void SetHoldingsImpl(Symbol symbol, decimal orderQuantity, bool liquidateExistingHoldings = false, string tag = null, IOrderProperties orderProperties = null)
private List<OrderTicket> SetHoldingsImpl(Symbol symbol, decimal orderQuantity, bool liquidateExistingHoldings = false, string tag = null, IOrderProperties orderProperties = null)
{
var orderTickets = new List<OrderTicket>();
//If they triggered a liquidate
if (liquidateExistingHoldings)
{
Liquidate(GetSymbolsToLiquidate([symbol]), tag: tag, orderProperties: orderProperties);
orderTickets = Liquidate(GetSymbolsToLiquidate([symbol]), tag: tag, orderProperties: orderProperties);
}

tag ??= "";
Expand All @@ -1435,19 +1444,22 @@ private void SetHoldingsImpl(Symbol symbol, decimal orderQuantity, bool liquidat
if (!Securities.TryGetValue(symbol, out security))
{
Error($"{symbol} not found in portfolio. Request this data when initializing the algorithm.");
return;
return orderTickets;
}

//Check whether the exchange is open to send a market order. If not, send a market on open order instead
OrderTicket ticket;
if (security.Exchange.ExchangeOpen)
{
MarketOrder(symbol, quantity, false, tag, orderProperties);
ticket = MarketOrder(symbol, quantity, false, tag, orderProperties);
}
else
{
MarketOnOpenOrder(symbol, quantity, tag, orderProperties);
ticket = MarketOnOpenOrder(symbol, quantity, tag, orderProperties);
}
orderTickets.Add(ticket);
}
return orderTickets;
}

/// <summary>
Expand Down
55 changes: 55 additions & 0 deletions Tests/Algorithm/AlgorithmTradingTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
using QuantConnect.Data;
using QuantConnect.Indicators;
using Python.Runtime;
using QuantConnect.Algorithm.Framework.Portfolio;

namespace QuantConnect.Tests.Algorithm
{
Expand Down Expand Up @@ -1320,6 +1321,51 @@ public void SetHoldings_Long_ToZero_RoundOff()
// Assert.AreEqual(2500, actual);
//}

[TestCaseSource(nameof(SetHoldingReturnsOrderTicketsTestCases))]
public void TestSetHoldingReturnsOrderTickets(IEnumerable<Symbol> symbols, bool liquidateExistingHoldings, int expectedOrderTickets, string tag)
{
symbols = symbols.ToList();
// Initialize the algorithm and add equities to the portfolio
var algo = GetAlgorithm(out _, 1, 0);
var appl = algo.AddEquity("AAPL");
var spy = algo.AddEquity("SPY");
var ibm = algo.AddEquity("IBM");

// Update prices and set initial holdings for the equities
Update(appl, 100);
Update(spy, 200);
Update(ibm, 300);
appl.Holdings.SetHoldings(25, 3);
spy.Holdings.SetHoldings(25, 3);
ibm.Holdings.SetHoldings(25, 3);

List<OrderTicket> orderTickets;
if (symbols.Count() > 1)
{
// Handle multiple symbols by creating portfolio targets
var portfolioTargets = new List<PortfolioTarget>();
foreach (var symbol in symbols)
{
portfolioTargets.Add(new PortfolioTarget(symbol, 0.5m));
}
orderTickets = algo.SetHoldings(portfolioTargets, liquidateExistingHoldings, tag);
}
else
{
// Handle a single symbol or no symbols
if (symbols.Any())
{
orderTickets = algo.SetHoldings(symbols.First(), 1, liquidateExistingHoldings, tag);
}
else
{
orderTickets = algo.SetHoldings(new List<PortfolioTarget>(), liquidateExistingHoldings, tag);
}
}
// Assert the expected number of order tickets
Assert.AreEqual(expectedOrderTickets, orderTickets.Count);
}

[Test]
public void OrderQuantityConversionTest()
{
Expand Down Expand Up @@ -1821,5 +1867,14 @@ private bool HasSufficientBuyingPowerForOrder(decimal orderQuantity, Security se
new object[] { Language.CSharp, null, false, null },
new object[] { Language.Python, null, false, null }
};
private static object[] SetHoldingReturnsOrderTicketsTestCases =
{
new object[] { new List<Symbol>(), true, 3, "(Empty, true)"},
new object[] { new List<Symbol>(), false, 0, "(Empty, false)" },
new object[] { new List<Symbol>() { Symbols.IBM }, true, 3, "(OneSymbol, true)" },
new object[] { new List<Symbol>() { Symbols.IBM }, false, 1, "(OneSymbol, false)" },
new object[] { new List<Symbol>() { Symbols.AAPL, Symbols.SPY }, true, 3, "(MultipleSymbols, true)" },
new object[] { new List<Symbol>() { Symbols.AAPL, Symbols.SPY }, false, 2, "(MultipleSymbols, false)" },
};
}
}