Improves Null Key Handling in Python

Common/ExtendedDictionary.cs

@@ -67,7 +67,7 @@ public virtual void Clear()
         /// <returns>true if the dictionary contains an element with the specified key; otherwise, false.</returns>
         public virtual bool ContainsKey(TKey key)
         {
-            return TryGetValue(key, out _);
+            return key != null && TryGetValue(key, out _);
         }
 
         /// <summary>
@@ -156,46 +156,45 @@ public PyDict fromkeys(TKey[] sequence)
         /// Each element of the newly created dictionary is set to the provided value.</returns>
         public PyDict fromkeys(TKey[] sequence, TValue value)
         {
-            using (Py.GIL())
+            using var _ = Py.GIL();
+            var pyDict = new PyDict();
+            foreach (var key in sequence.Where(k => k != null))
             {
-                var dict = new PyDict();
-                foreach (var key in sequence)
-                {
-                    var pyValue = get(key, value);
-                    dict.SetItem(key.ToPython(), pyValue.ToPython());
-                }
-                return dict;
+                value = get(key, value);
+                using var pyKey = key.ToPython();
+                using var pyValue = value.ToPython();
+                pyDict.SetItem(pyKey, pyValue);
             }
+            return pyDict;
         }
 
         /// <summary>
         /// Returns the value for the specified key if key is in dictionary.
         /// </summary>
         /// <param name="key">key to be searched in the dictionary</param>
         /// <returns>The value for the specified key if key is in dictionary.
-        /// None if the key is not found and value is not specified.</returns>
+        /// None if the key is not found, or if the key is None.</returns>
         public TValue get(TKey key)
         {
-            TValue data;
-            TryGetValue(key, out data);
-            return data;
+            return get(key, default);
         }
 
         /// <summary>
         /// Returns the value for the specified key if key is in dictionary.
         /// </summary>
         /// <param name="key">key to be searched in the dictionary</param>
-        /// <param name="value">Value to be returned if the key is not found. The default value is null.</param>
+        /// <param name="default_value">Value to be returned if the key is not found or if the key is None.</param>
         /// <returns>The value for the specified key if key is in dictionary.
-        /// value if the key is not found and value is specified.</returns>
-        public TValue get(TKey key, TValue value)
+        /// default_value if the key is not found, or if the key is None.</returns>
+        public TValue get(TKey key, TValue default_value)
         {
-            TValue data;
-            if (TryGetValue(key, out data))
+            if (key == null) return default_value;
+
+            if (TryGetValue(key, out TValue data))
             {
                 return data;
             }
-            return value;
+            return default_value;
         }
 
         /// <summary>
@@ -204,18 +203,16 @@ public TValue get(TKey key, TValue value)
         /// <returns>Returns a view object that displays a list of a given dictionary's (key, value) tuple pair.</returns>
         public PyList items()
         {
-            using (Py.GIL())
+            using var _ = Py.GIL();
+            var pyList = new PyList();
+            foreach (var (key, value) in GetItems())
             {
-                var pyList = new PyList();
-                foreach (var (key, value) in GetItems())
-                {
-                    using var pyKey = key.ToPython();
-                    using var pyValue = value.ToPython();
-                    using var pyKvp = new PyTuple([pyKey, pyValue]);
-                    pyList.Append(pyKvp);
-                }
-                return pyList;
+                using var pyKey = key.ToPython();
+                using var pyValue = value.ToPython();
+                using var pyKvp = new PyTuple([pyKey, pyValue]);
+                pyList.Append(pyKvp);
             }
+            return pyList;
         }
 
         /// <summary>
@@ -249,8 +246,7 @@ public TValue setdefault(TKey key)
         /// default_value if key is not in the dictionary and default_value is specified</returns>
         public TValue setdefault(TKey key, TValue default_value)
         {
-            TValue data;
-            if (TryGetValue(key, out data))
+            if (TryGetValue(key, out TValue data))
             {
                 return data;
             }
@@ -272,7 +268,15 @@ public TValue setdefault(TKey key, TValue default_value)
         /// If key is not found - KeyError exception is raised</returns>
         public TValue pop(TKey key)
         {
-            return pop(key, default);
+            if (key == null)
+            {
+                throw new KeyNotFoundException(Messages.ExtendedDictionary.KeyNotFoundDueToNone);
+            }
+            if (TryGetValue(key, out TValue data))
+            {
+                Remove(key);
+            }
+            return data;
         }
 
         /// <summary>
@@ -284,8 +288,7 @@ public TValue pop(TKey key)
         /// If key is not found - value specified as the second argument(default)</returns>
         public TValue pop(TKey key, TValue default_value)
         {
-            TValue data;
-            if (TryGetValue(key, out data))
+            if (key != null && TryGetValue(key, out TValue data))
             {
                 Remove(key);
                 return data;

Common/Messages/Messages.QuantConnect.cs

@@ -113,8 +113,7 @@ public static class Candlestick
             [MethodImpl(MethodImplOptions.AggressiveInlining)]
             public static string ToString(QuantConnect.Candlestick instance)
             {
-                return Invariant($@"{instance.Time:o} - (O:{instance.Open} H: {instance.High} L: {
-                    instance.Low} C: {instance.Close})");
+                return Invariant($@"{instance.Time:o} - (O:{instance.Open} H: {instance.High} L: {instance.Low} C: {instance.Close})");
             }
         }
 
@@ -155,6 +154,11 @@ public static class ExtendedDictionary
             public static string IndexerBySymbolNotImplemented =
                 "Types deriving from 'ExtendedDictionary' must implement the 'T this[Symbol] method.";
 
+            /// <summary>
+            /// 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
+            /// </summary>
+            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.";
+
             /// <summary>
             /// Returns a string message saying Clear/clear method call is an invalid operation. It also says that the given instance
             /// is a read-only collection
@@ -338,8 +342,7 @@ public static string TypeIsNotBaseData(Type type)
             [MethodImpl(MethodImplOptions.AggressiveInlining)]
             public static string CannotCastNonFiniteFloatingPointValueToDecimal(double input)
             {
-                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.");
+                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.");
             }
 
             /// <summary>
@@ -471,8 +474,7 @@ public static string ToString(QuantConnect.Holding instance)
                 {
                     currencySymbol = "$";
                 }
-                var value = Invariant($@"{instance.Symbol?.Value}: {instance.Quantity} @ {
-                    currencySymbol}{instance.AveragePrice} - Market: {currencySymbol}{instance.MarketPrice}");
+                var value = Invariant($@"{instance.Symbol?.Value}: {instance.Quantity} @ {currencySymbol}{instance.AveragePrice} - Market: {currencySymbol}{instance.MarketPrice}");
 
                 if (instance.ConversionRate.HasValue && instance.ConversionRate != 1m)
                 {
@@ -526,8 +528,7 @@ public static string MemoryUsageOver80Percent(double lastSample)
             public static string MemoryUsageInfo(string memoryUsed, string lastSample, string memoryUsedByApp, TimeSpan currentTimeStepElapsed,
                 int cpuUsage)
             {
-                return Invariant($@"Used: {memoryUsed}, Sample: {lastSample}, App: {memoryUsedByApp}, CurrentTimeStepElapsed: {
-                    currentTimeStepElapsed:mm':'ss'.'fff}. CPU: {cpuUsage}%");
+                return Invariant($@"Used: {memoryUsed}, Sample: {lastSample}, App: {memoryUsedByApp}, CurrentTimeStepElapsed: {currentTimeStepElapsed:mm':'ss'.'fff}. CPU: {cpuUsage}%");
             }
 
             /// <summary>
@@ -537,8 +538,7 @@ public static string MemoryUsageInfo(string memoryUsed, string lastSample, strin
             [MethodImpl(MethodImplOptions.AggressiveInlining)]
             public static string MemoryUsageMonitorTaskTimedOut(TimeSpan timeout)
             {
-                return $@"Execution Security Error: Operation timed out - {
-                    timeout.TotalMinutes.ToStringInvariant()} minutes max. Check for recursive loops.";
+                return $@"Execution Security Error: Operation timed out - {timeout.TotalMinutes.ToStringInvariant()} minutes max. Check for recursive loops.";
             }
         }
 
@@ -724,8 +724,7 @@ public static string ErrorParsingSecurityIdentifier(string value, Exception exce
             [MethodImpl(MethodImplOptions.AggressiveInlining)]
             public static string MarketNotFound(string market)
             {
-                return $@"The specified market wasn't found in the markets lookup. Requested: {
-                    market}. You can add markets by calling QuantConnect.Market.Add(string,int)";
+                return $@"The specified market wasn't found in the markets lookup. Requested: {market}. You can add markets by calling QuantConnect.Market.Add(string,int)";
             }
         }
 
@@ -940,9 +939,7 @@ public static class TradingCalendar
             [MethodImpl(MethodImplOptions.AggressiveInlining)]
             public static string InvalidTotalDays(int totalDays)
             {
-                return Invariant($@"Total days is negative ({
-                    totalDays
-                    }), indicating reverse start and end times. Check your usage of TradingCalendar to ensure proper arrangement of variables");
+                return Invariant($@"Total days is negative ({totalDays}), indicating reverse start and end times. Check your usage of TradingCalendar to ensure proper arrangement of variables");
             }
         }
     }

Tests/Common/ExtendedDictionaryTests.cs

@@ -18,6 +18,7 @@
 using QuantConnect.Statistics;
 using System.Collections.Generic;
 using System.Linq;
+using QuantConnect.Data.Market;
 
 namespace QuantConnect.Tests.Common
 {
@@ -118,6 +119,60 @@ def pop(dictionary, key):
             Assert.IsFalse(module.InvokeMethod("contains", pyDict, pyExistingKey).As<bool>());
         }
 
+        [Test]
+        public void NullKeyIsHandledGracefully()
+        {
+            using var _ = Py.GIL();
+
+            var module = PyModule.FromString("NullKeyIsHandledGracefully",
+                    @"
+def contains(dictionary, key):
+    return key in dictionary
+
+def get(dictionary, key):
+    return dictionary.get(key)
+
+def get_default(dictionary, key, default_value):
+    return dictionary.get(key, default_value)
+
+def pop(dictionary, key):
+    return dictionary.pop(key)
+
+def pop_default(dictionary, key, default_value):
+    return dictionary.pop(key, default_value)
+");
+
+            var tradeBarA = new TradeBar { Close = 1 };
+            var tradeBarB = new TradeBar { Close = 2 };
+            var dict = new TestDictionary<string, TradeBar>
+            {
+                ["a"] = tradeBarA,
+                ["b"] = tradeBarB
+            };
+            using var pyDict = dict.ToPython();
+
+            // contains with None key should return False
+            Assert.IsFalse(module.InvokeMethod("contains", pyDict, PyObject.None).As<bool>());
+
+            // get with None key should return None
+            Assert.IsTrue(module.InvokeMethod("get", pyDict, PyObject.None).IsNone());
+
+            // get with None key and default value should return the default value
+            using var pyDefault = tradeBarA.ToPython();
+            Assert.AreEqual(tradeBarA.Close, module.InvokeMethod("get_default", pyDict, PyObject.None, pyDefault).As<TradeBar>().Close);
+
+            // pop with None key and default value should return the default value
+            using var pyPopDefault = tradeBarB.ToPython();
+            Assert.AreEqual(tradeBarB.Close, module.InvokeMethod("pop_default", pyDict, PyObject.None, pyPopDefault).As<TradeBar>().Close);
+
+            // Dictionary should not be modified after pop with None key
+            Assert.AreEqual(2, dict.Count);
+
+            // pop with None key without default should raise KeyNotFoundException
+            var exception = Assert.Throws<ClrBubbledException>(() => module.InvokeMethod("pop", pyDict, PyObject.None));
+            Assert.IsInstanceOf<KeyNotFoundException>(exception.InnerException);
+        }
+
         [Test]
         public void SymbolKeyCanBeIndexedWithStrings()
         {