Skip to content

Commit 1813a60

Browse files
AlexCatarinoAlexCatarino
authored
Improves Null Key Handling in Python (#9368)
* Improves Null Key Handling in Python We aim to mimic the following behavior: ```python >>> d = {1:2} >>> None in d False >>> d.get(None) is None True >>> d.pop(None) Traceback (most recent call last): File "<stdin>", line 1, in <module> KeyError: None >>> d.pop(None, None) is None True ``` * Reverts dispose change
1 parent bc02b46 commit 1813a60

3 files changed

Lines changed: 102 additions & 49 deletions

File tree

‎Common/ExtendedDictionary.cs‎

Lines changed: 35 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ public virtual void Clear()
6767
/// <returns>true if the dictionary contains an element with the specified key; otherwise, false.</returns>
6868
public virtual bool ContainsKey(TKey key)
6969
{
70-
return TryGetValue(key, out _);
70+
return key != null && TryGetValue(key, out _);
7171
}
7272

7373
/// <summary>
@@ -156,46 +156,43 @@ public PyDict fromkeys(TKey[] sequence)
156156
/// Each element of the newly created dictionary is set to the provided value.</returns>
157157
public PyDict fromkeys(TKey[] sequence, TValue value)
158158
{
159-
using (Py.GIL())
159+
using var _ = Py.GIL();
160+
var pyDict = new PyDict();
161+
foreach (var key in sequence.Where(k => k != null))
160162
{
161-
var dict = new PyDict();
162-
foreach (var key in sequence)
163-
{
164-
var pyValue = get(key, value);
165-
dict.SetItem(key.ToPython(), pyValue.ToPython());
166-
}
167-
return dict;
163+
var pyValue = get(key, value);
164+
pyDict.SetItem(key.ToPython(), pyValue.ToPython());
168165
}
166+
return pyDict;
169167
}
170168

171169
/// <summary>
172170
/// Returns the value for the specified key if key is in dictionary.
173171
/// </summary>
174172
/// <param name="key">key to be searched in the dictionary</param>
175173
/// <returns>The value for the specified key if key is in dictionary.
176-
/// None if the key is not found and value is not specified.</returns>
174+
/// None if the key is not found, or if the key is None.</returns>
177175
public TValue get(TKey key)
178176
{
179-
TValue data;
180-
TryGetValue(key, out data);
181-
return data;
177+
return get(key, default);
182178
}
183179

184180
/// <summary>
185181
/// Returns the value for the specified key if key is in dictionary.
186182
/// </summary>
187183
/// <param name="key">key to be searched in the dictionary</param>
188-
/// <param name="value">Value to be returned if the key is not found. The default value is null.</param>
184+
/// <param name="default_value">Value to be returned if the key is not found or if the key is None.</param>
189185
/// <returns>The value for the specified key if key is in dictionary.
190-
/// value if the key is not found and value is specified.</returns>
191-
public TValue get(TKey key, TValue value)
186+
/// default_value if the key is not found, or if the key is None.</returns>
187+
public TValue get(TKey key, TValue default_value)
192188
{
193-
TValue data;
194-
if (TryGetValue(key, out data))
189+
if (key == null) return default_value;
190+
191+
if (TryGetValue(key, out TValue data))
195192
{
196193
return data;
197194
}
198-
return value;
195+
return default_value;
199196
}
200197

201198
/// <summary>
@@ -204,18 +201,16 @@ public TValue get(TKey key, TValue value)
204201
/// <returns>Returns a view object that displays a list of a given dictionary's (key, value) tuple pair.</returns>
205202
public PyList items()
206203
{
207-
using (Py.GIL())
204+
using var _ = Py.GIL();
205+
var pyList = new PyList();
206+
foreach (var (key, value) in GetItems())
208207
{
209-
var pyList = new PyList();
210-
foreach (var (key, value) in GetItems())
211-
{
212-
using var pyKey = key.ToPython();
213-
using var pyValue = value.ToPython();
214-
using var pyKvp = new PyTuple([pyKey, pyValue]);
215-
pyList.Append(pyKvp);
216-
}
217-
return pyList;
208+
using var pyKey = key.ToPython();
209+
using var pyValue = value.ToPython();
210+
using var pyKvp = new PyTuple([pyKey, pyValue]);
211+
pyList.Append(pyKvp);
218212
}
213+
return pyList;
219214
}
220215

221216
/// <summary>
@@ -249,8 +244,7 @@ public TValue setdefault(TKey key)
249244
/// default_value if key is not in the dictionary and default_value is specified</returns>
250245
public TValue setdefault(TKey key, TValue default_value)
251246
{
252-
TValue data;
253-
if (TryGetValue(key, out data))
247+
if (TryGetValue(key, out TValue data))
254248
{
255249
return data;
256250
}
@@ -272,7 +266,15 @@ public TValue setdefault(TKey key, TValue default_value)
272266
/// If key is not found - KeyError exception is raised</returns>
273267
public TValue pop(TKey key)
274268
{
275-
return pop(key, default);
269+
if (key == null)
270+
{
271+
throw new KeyNotFoundException(Messages.ExtendedDictionary.KeyNotFoundDueToNone);
272+
}
273+
if (TryGetValue(key, out TValue data))
274+
{
275+
Remove(key);
276+
}
277+
return data;
276278
}
277279

278280
/// <summary>
@@ -284,8 +286,7 @@ public TValue pop(TKey key)
284286
/// If key is not found - value specified as the second argument(default)</returns>
285287
public TValue pop(TKey key, TValue default_value)
286288
{
287-
TValue data;
288-
if (TryGetValue(key, out data))
289+
if (key != null && TryGetValue(key, out TValue data))
289290
{
290291
Remove(key);
291292
return data;

‎Common/Messages/Messages.QuantConnect.cs‎

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,7 @@ public static class Candlestick
113113
[MethodImpl(MethodImplOptions.AggressiveInlining)]
114114
public static string ToString(QuantConnect.Candlestick instance)
115115
{
116-
return Invariant($@"{instance.Time:o} - (O:{instance.Open} H: {instance.High} L: {
117-
instance.Low} C: {instance.Close})");
116+
return Invariant($@"{instance.Time:o} - (O:{instance.Open} H: {instance.High} L: {instance.Low} C: {instance.Close})");
118117
}
119118
}
120119

@@ -155,6 +154,11 @@ public static class ExtendedDictionary
155154
public static string IndexerBySymbolNotImplemented =
156155
"Types deriving from 'ExtendedDictionary' must implement the 'T this[Symbol] method.";
157156

157+
/// <summary>
158+
/// Returns a string with the error message we receive from Python when we try to pop a key with a null value in the ExtendedDictionary. It also shows a recommendation for solving this problem
159+
/// </summary>
160+
public static string KeyNotFoundDueToNone = $"KeyError: None. Please check if the key is None before trying to access it or use data.pop(key, default) to return a default value instead of raising an exception.";
161+
158162
/// <summary>
159163
/// Returns a string message saying Clear/clear method call is an invalid operation. It also says that the given instance
160164
/// is a read-only collection
@@ -338,8 +342,7 @@ public static string TypeIsNotBaseData(Type type)
338342
[MethodImpl(MethodImplOptions.AggressiveInlining)]
339343
public static string CannotCastNonFiniteFloatingPointValueToDecimal(double input)
340344
{
341-
return Invariant($@"It is not possible to cast a non-finite floating-point value ({
342-
input}) as decimal. Please review math operations and verify the result is valid.");
345+
return Invariant($@"It is not possible to cast a non-finite floating-point value ({input}) as decimal. Please review math operations and verify the result is valid.");
343346
}
344347

345348
/// <summary>
@@ -471,8 +474,7 @@ public static string ToString(QuantConnect.Holding instance)
471474
{
472475
currencySymbol = "$";
473476
}
474-
var value = Invariant($@"{instance.Symbol?.Value}: {instance.Quantity} @ {
475-
currencySymbol}{instance.AveragePrice} - Market: {currencySymbol}{instance.MarketPrice}");
477+
var value = Invariant($@"{instance.Symbol?.Value}: {instance.Quantity} @ {currencySymbol}{instance.AveragePrice} - Market: {currencySymbol}{instance.MarketPrice}");
476478

477479
if (instance.ConversionRate.HasValue && instance.ConversionRate != 1m)
478480
{
@@ -526,8 +528,7 @@ public static string MemoryUsageOver80Percent(double lastSample)
526528
public static string MemoryUsageInfo(string memoryUsed, string lastSample, string memoryUsedByApp, TimeSpan currentTimeStepElapsed,
527529
int cpuUsage)
528530
{
529-
return Invariant($@"Used: {memoryUsed}, Sample: {lastSample}, App: {memoryUsedByApp}, CurrentTimeStepElapsed: {
530-
currentTimeStepElapsed:mm':'ss'.'fff}. CPU: {cpuUsage}%");
531+
return Invariant($@"Used: {memoryUsed}, Sample: {lastSample}, App: {memoryUsedByApp}, CurrentTimeStepElapsed: {currentTimeStepElapsed:mm':'ss'.'fff}. CPU: {cpuUsage}%");
531532
}
532533

533534
/// <summary>
@@ -537,8 +538,7 @@ public static string MemoryUsageInfo(string memoryUsed, string lastSample, strin
537538
[MethodImpl(MethodImplOptions.AggressiveInlining)]
538539
public static string MemoryUsageMonitorTaskTimedOut(TimeSpan timeout)
539540
{
540-
return $@"Execution Security Error: Operation timed out - {
541-
timeout.TotalMinutes.ToStringInvariant()} minutes max. Check for recursive loops.";
541+
return $@"Execution Security Error: Operation timed out - {timeout.TotalMinutes.ToStringInvariant()} minutes max. Check for recursive loops.";
542542
}
543543
}
544544

@@ -724,8 +724,7 @@ public static string ErrorParsingSecurityIdentifier(string value, Exception exce
724724
[MethodImpl(MethodImplOptions.AggressiveInlining)]
725725
public static string MarketNotFound(string market)
726726
{
727-
return $@"The specified market wasn't found in the markets lookup. Requested: {
728-
market}. You can add markets by calling QuantConnect.Market.Add(string,int)";
727+
return $@"The specified market wasn't found in the markets lookup. Requested: {market}. You can add markets by calling QuantConnect.Market.Add(string,int)";
729728
}
730729
}
731730

@@ -940,9 +939,7 @@ public static class TradingCalendar
940939
[MethodImpl(MethodImplOptions.AggressiveInlining)]
941940
public static string InvalidTotalDays(int totalDays)
942941
{
943-
return Invariant($@"Total days is negative ({
944-
totalDays
945-
}), indicating reverse start and end times. Check your usage of TradingCalendar to ensure proper arrangement of variables");
942+
return Invariant($@"Total days is negative ({totalDays}), indicating reverse start and end times. Check your usage of TradingCalendar to ensure proper arrangement of variables");
946943
}
947944
}
948945
}

‎Tests/Common/ExtendedDictionaryTests.cs‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
using QuantConnect.Statistics;
1919
using System.Collections.Generic;
2020
using System.Linq;
21+
using QuantConnect.Data.Market;
2122

2223
namespace QuantConnect.Tests.Common
2324
{
@@ -118,6 +119,60 @@ def pop(dictionary, key):
118119
Assert.IsFalse(module.InvokeMethod("contains", pyDict, pyExistingKey).As<bool>());
119120
}
120121

122+
[Test]
123+
public void NullKeyIsHandledGracefully()
124+
{
125+
using var _ = Py.GIL();
126+
127+
var module = PyModule.FromString("NullKeyIsHandledGracefully",
128+
@"
129+
def contains(dictionary, key):
130+
return key in dictionary
131+
132+
def get(dictionary, key):
133+
return dictionary.get(key)
134+
135+
def get_default(dictionary, key, default_value):
136+
return dictionary.get(key, default_value)
137+
138+
def pop(dictionary, key):
139+
return dictionary.pop(key)
140+
141+
def pop_default(dictionary, key, default_value):
142+
return dictionary.pop(key, default_value)
143+
");
144+
145+
var tradeBarA = new TradeBar { Close = 1 };
146+
var tradeBarB = new TradeBar { Close = 2 };
147+
var dict = new TestDictionary<string, TradeBar>
148+
{
149+
["a"] = tradeBarA,
150+
["b"] = tradeBarB
151+
};
152+
using var pyDict = dict.ToPython();
153+
154+
// contains with None key should return False
155+
Assert.IsFalse(module.InvokeMethod("contains", pyDict, PyObject.None).As<bool>());
156+
157+
// get with None key should return None
158+
Assert.IsTrue(module.InvokeMethod("get", pyDict, PyObject.None).IsNone());
159+
160+
// get with None key and default value should return the default value
161+
using var pyDefault = tradeBarA.ToPython();
162+
Assert.AreEqual(tradeBarA.Close, module.InvokeMethod("get_default", pyDict, PyObject.None, pyDefault).As<TradeBar>().Close);
163+
164+
// pop with None key and default value should return the default value
165+
using var pyPopDefault = tradeBarB.ToPython();
166+
Assert.AreEqual(tradeBarB.Close, module.InvokeMethod("pop_default", pyDict, PyObject.None, pyPopDefault).As<TradeBar>().Close);
167+
168+
// Dictionary should not be modified after pop with None key
169+
Assert.AreEqual(2, dict.Count);
170+
171+
// pop with None key without default should raise KeyNotFoundException
172+
var exception = Assert.Throws<ClrBubbledException>(() => module.InvokeMethod("pop", pyDict, PyObject.None));
173+
Assert.IsInstanceOf<KeyNotFoundException>(exception.InnerException);
174+
}
175+
121176
[Test]
122177
public void SymbolKeyCanBeIndexedWithStrings()
123178
{

0 commit comments

Comments
 (0)