Skip to content

Commit a0be00d

Browse files
committed
Throw clear error when AddData is called with a non-class first argument from Python
Calls like self.add_data("VIX", Resolution.DAILY) routed through CreateType, which silently built a dynamic assembly named after the string and returned a fake type whose activator factory tried to invoke the str like a function. The downstream 'str' object is not callable PythonException surfaced confusingly (and could manifest as an apparent hang depending on where it was caught), making the actual mistake hard to diagnose. Validate the PyObject up front via TryCreateType in the AddData(PyObject, ...) entry points and throw an ArgumentException pointing the user at AddEquity / AddForex / etc. when the argument is not a custom data class.
1 parent 3806e81 commit a0be00d

3 files changed

Lines changed: 45 additions & 3 deletions

File tree

Algorithm/QCAlgorithm.Python.cs

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ public Security AddData(PyObject type, Symbol underlying, Resolution? resolution
109109
[DocumentationAttribute(AddingData)]
110110
public Security AddData(PyObject type, string ticker, Resolution? resolution, DateTimeZone timeZone, bool fillForward = false, decimal leverage = 1.0m)
111111
{
112-
return AddData(type.CreateType(), ticker, resolution, timeZone, fillForward, leverage);
112+
return AddData(GetCustomDataType(type), ticker, resolution, timeZone, fillForward, leverage);
113113
}
114114

115115
/// <summary>
@@ -135,7 +135,7 @@ public Security AddData(PyObject type, string ticker, Resolution? resolution, Da
135135
[DocumentationAttribute(AddingData)]
136136
public Security AddData(PyObject type, Symbol underlying, Resolution? resolution, DateTimeZone timeZone, bool fillForward = false, decimal leverage = 1.0m)
137137
{
138-
return AddData(type.CreateType(), underlying, resolution, timeZone, fillForward, leverage);
138+
return AddData(GetCustomDataType(type), underlying, resolution, timeZone, fillForward, leverage);
139139
}
140140

141141
/// <summary>
@@ -219,7 +219,7 @@ public Security AddData(Type dataType, Symbol underlying, Resolution? resolution
219219
public Security AddData(PyObject type, string ticker, SymbolProperties properties, SecurityExchangeHours exchangeHours, Resolution? resolution = null, bool fillForward = false, decimal leverage = 1.0m)
220220
{
221221
// Get the right key for storage of base type symbols
222-
var dataType = type.CreateType();
222+
var dataType = GetCustomDataType(type);
223223
var key = SecurityIdentifier.GenerateBaseSymbol(dataType, ticker);
224224

225225
// Add entries to our Symbol Properties DB and MarketHours DB
@@ -284,6 +284,21 @@ private Security AddDataImpl(Type dataType, Symbol symbol, Resolution? resolutio
284284
return AddToUserDefinedUniverse(security, new List<SubscriptionDataConfig> { config });
285285
}
286286

287+
/// <summary>
288+
/// Resolves the custom data <see cref="Type"/> from the PyObject argument of <see cref="AddData(PyObject, string, Resolution?)"/> and overloads,
289+
/// throwing a clear exception if the caller passed something other than a custom data class (e.g. a string ticker).
290+
/// </summary>
291+
private static Type GetCustomDataType(PyObject type)
292+
{
293+
if (type.TryCreateType(out var dataType))
294+
{
295+
return dataType;
296+
}
297+
298+
using var _ = Py.GIL();
299+
throw new ArgumentException(Messages.QCAlgorithm.AddDataInvalidPyObjectType(type.Repr()));
300+
}
301+
287302
/// <summary>
288303
/// Creates a new universe and adds it to the algorithm. This is for coarse fundamental US Equity data and
289304
/// will be executed on day changes in the NewYork time zone (<see cref="TimeZones.NewYork"/>)

Common/Messages/Messages.Algorithm.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,16 @@ public static string SetWarmupAlreadyInitialized()
8989
{
9090
return $"{AlgorithmPrefix()}.{FormatCode("SetWarmup")}(): This method cannot be used after algorithm initialized";
9191
}
92+
93+
/// <summary>
94+
/// Returns a string message saying the first argument to AddData must be a custom data class
95+
/// </summary>
96+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
97+
public static string AddDataInvalidPyObjectType(string repr)
98+
{
99+
return $"{AlgorithmPrefix()}.{FormatCode("AddData")}(): the first argument must be a custom data type (a Python class deriving from {FormatCode("PythonData")} or a CLR {FormatCode("BaseData")} type), but received {repr}. " +
100+
$"To subscribe to built-in asset classes use {FormatCode("AddEquity")}, {FormatCode("AddForex")}, {FormatCode("AddCfd")}, {FormatCode("AddCrypto")}, {FormatCode("AddFuture")}, {FormatCode("AddOption")} or {FormatCode("AddSecurity")} instead.";
101+
}
92102
}
93103

94104
/// <summary>

Tests/Algorithm/AlgorithmAddDataTests.cs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
using Newtonsoft.Json;
2020
using NodaTime;
2121
using NUnit.Framework;
22+
using Python.Runtime;
2223
using QuantConnect.Algorithm;
2324
using QuantConnect.Algorithm.Framework.Selection;
2425
using QuantConnect.Algorithm.Selection;
@@ -637,6 +638,22 @@ public void AddingInvalidDataTypeThrows()
637638
DateTimeZone.Utc));
638639
}
639640

641+
[Test]
642+
public void AddDataWithStringAsTypeArgumentThrowsClearError()
643+
{
644+
var qcAlgorithm = new QCAlgorithm();
645+
qcAlgorithm.SubscriptionManager.SetDataManager(new DataManagerStub(qcAlgorithm));
646+
647+
using var _ = Py.GIL();
648+
using var pyTicker = "VIX".ToPython();
649+
650+
// Passing a string instead of a custom data class as the first argument used to silently build a
651+
// dynamic assembly named after the string and later hang/fail downstream. It must now throw.
652+
var ex = Assert.Throws<ArgumentException>(() => qcAlgorithm.AddData(pyTicker, "VIX", Resolution.Daily));
653+
StringAssert.Contains("AddData", ex.Message);
654+
StringAssert.Contains("AddEquity", ex.Message);
655+
}
656+
640657
[Test]
641658
public void AppendsCustomDataTypeName_ToSecurityIdentifierSymbol()
642659
{

0 commit comments

Comments
 (0)