Skip to content

Commit 7a9778f

Browse files
AlexCatarinoAlexCatarino
committed
Addresses Peer-Review
- Do not reuse parameter variables `start` and `end`. Create new variables with meaningful names and rename them to `startUtc` and `endUtc`. - Use `EachTradeableDayInTimeZone` to calculate `tradableDates`. - Adds another test/assertion in the regression algorithms to ensure tests in the scheduled event were performed.
1 parent 8b1afdb commit 7a9778f

3 files changed

Lines changed: 33 additions & 19 deletions

File tree

‎Algorithm.CSharp/BasicTemplateFuturesHistoryAlgorithm.cs‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
using QuantConnect.Orders;
2121
using QuantConnect.Securities;
2222
using QuantConnect.Data.UniverseSelection;
23-
using QuantConnect.Util;
2423
using QuantConnect.Interfaces;
2524
using System.Collections.Generic;
2625

@@ -44,6 +43,8 @@ public class BasicTemplateFuturesHistoryAlgorithm : QCAlgorithm, IRegressionAlgo
4443
Futures.Metals.Gold,
4544
};
4645

46+
private int _successCount = 0;
47+
4748
public override void Initialize()
4849
{
4950
SetStartDate(2013, 10, 8);
@@ -64,10 +65,19 @@ public override void Initialize()
6465
private void MakeHistoryCall()
6566
{
6667
var history = History(10, Resolution.Minute);
67-
if (history.IsNullOrEmpty())
68+
if (history.Count() < 10)
6869
{
6970
throw new Exception($"Empty history at {Time}");
7071
}
72+
_successCount++;
73+
}
74+
75+
public override void OnEndOfAlgorithm()
76+
{
77+
if (_successCount < 49)
78+
{
79+
throw new Exception($"Scheduled Event did not assert history call as many times as expected: {_successCount}/49");
80+
}
7181
}
7282

7383
/// <summary>

‎Algorithm.Python/BasicTemplateFuturesHistoryAlgorithm.py‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,17 @@ def Initialize(self):
5050
self.SetBenchmark(lambda x: 1000000)
5151

5252
self.Schedule.On(self.DateRules.EveryDay(), self.TimeRules.Every(timedelta(hours=1)), self.MakeHistoryCall)
53+
self.successCount = 0
5354

5455
def MakeHistoryCall(self):
5556
history = self.History(self.Securities.keys(), 10, Resolution.Minute)
56-
if history.empty:
57+
if len(history) < 10:
5758
raise Exception(f'Empty history at {self.Time}')
59+
self.successCount += 1
60+
61+
def OnEndOfAlgorithm(self):
62+
if self.successCount < 49:
63+
raise Exception(f'Scheduled Event did not assert history call as many times as expected: {_successCount}/49')
5864

5965
def OnData(self,slice):
6066
if self.Portfolio.Invested: return

‎Engine/HistoricalData/SubscriptionDataReaderHistoryProvider.cs‎

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -77,16 +77,11 @@ public override IEnumerable<Slice> GetHistory(IEnumerable<HistoryRequest> reques
7777
/// <summary>
7878
/// Creates a subscription to process the request
7979
/// </summary>
80-
protected virtual Subscription CreateSubscription(HistoryRequest request, DateTime start, DateTime end)
80+
protected virtual Subscription CreateSubscription(HistoryRequest request, DateTime startUtc, DateTime endUtc)
8181
{
82-
// Tradable dates are defined with the data time zone to access the right source
83-
var tradableDates = Time.EachTradeableDay(request.ExchangeHours,
84-
start.ConvertFromUtc(request.DataTimeZone),
85-
end.ConvertFromUtc(request.DataTimeZone));
86-
8782
// data reader expects these values in local times
88-
start = start.ConvertFromUtc(request.ExchangeHours.TimeZone);
89-
end = end.ConvertFromUtc(request.ExchangeHours.TimeZone);
83+
var startTimeLocal = startUtc.ConvertFromUtc(request.ExchangeHours.TimeZone);
84+
var endTimeLocal = endUtc.ConvertFromUtc(request.ExchangeHours.TimeZone);
9085

9186
var config = new SubscriptionDataConfig(request.DataType,
9287
request.Symbol,
@@ -117,12 +112,15 @@ protected virtual Subscription CreateSubscription(HistoryRequest request, DateTi
117112
{
118113
mapFileResolver = _mapFileProvider.Get(config.Market);
119114
var mapFile = mapFileResolver.ResolveMapFile(config.Symbol.ID.Symbol, config.Symbol.ID.Date);
120-
config.MappedSymbol = mapFile.GetMappedSymbol(start, config.MappedSymbol);
115+
config.MappedSymbol = mapFile.GetMappedSymbol(startTimeLocal, config.MappedSymbol);
121116
}
122117

118+
// Tradable dates are defined with the data time zone to access the right source
119+
var tradableDates = Time.EachTradeableDayInTimeZone(request.ExchangeHours, startTimeLocal, endTimeLocal, request.DataTimeZone, request.IncludeExtendedMarketHours);
120+
123121
var dataReader = new SubscriptionDataReader(config,
124-
start,
125-
end,
122+
startTimeLocal,
123+
endTimeLocal,
126124
mapFileResolver,
127125
_factorFileProvider,
128126
tradableDates,
@@ -151,7 +149,7 @@ protected virtual Subscription CreateSubscription(HistoryRequest request, DateTi
151149
dataReader,
152150
mapFileResolver,
153151
false,
154-
start);
152+
startTimeLocal);
155153

156154
// optionally apply fill forward behavior
157155
if (request.FillForwardResolution.HasValue)
@@ -163,23 +161,23 @@ protected virtual Subscription CreateSubscription(HistoryRequest request, DateTi
163161
}
164162

165163
var readOnlyRef = Ref.CreateReadOnly(() => request.FillForwardResolution.Value.ToTimeSpan());
166-
reader = new FillForwardEnumerator(reader, security.Exchange, readOnlyRef, request.IncludeExtendedMarketHours, end, config.Increment, config.DataTimeZone, start);
164+
reader = new FillForwardEnumerator(reader, security.Exchange, readOnlyRef, request.IncludeExtendedMarketHours, endTimeLocal, config.Increment, config.DataTimeZone, startTimeLocal);
167165
}
168166

169167
// since the SubscriptionDataReader performs an any overlap condition on the trade bar's entire
170168
// range (time->end time) we can end up passing the incorrect data (too far past, possibly future),
171169
// so to combat this we deliberately filter the results from the data reader to fix these cases
172170
// which only apply to non-tick data
173171

174-
reader = new SubscriptionFilterEnumerator(reader, security, end);
172+
reader = new SubscriptionFilterEnumerator(reader, security, endTimeLocal);
175173
reader = new FilterEnumerator<BaseData>(reader, data =>
176174
{
177175
// allow all ticks
178176
if (config.Resolution == Resolution.Tick) return true;
179177
// filter out future data
180-
if (data.EndTime > end) return false;
178+
if (data.EndTime > endTimeLocal) return false;
181179
// filter out data before the start
182-
return data.EndTime > start;
180+
return data.EndTime > startTimeLocal;
183181
});
184182
var subscriptionRequest = new SubscriptionRequest(false, null, security, config, request.StartTimeUtc, request.EndTimeUtc);
185183

0 commit comments

Comments
 (0)