Skip to content

Commit

Permalink
ENH: Change open to midnight if using daily input data.
Browse files Browse the repository at this point in the history
  • Loading branch information
@twiecki
twiecki committed Jun 29, 2014
1 parent 295a614 commit 5f01e11
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 6 deletions.
5 changes: 4 additions & 1 deletion zipline/algorithm.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -799,7 +799,10 @@ def raw_orders(self):
@api_method
def add_history(self, bar_count, frequency, field,
ffill=True):
history_spec = HistorySpec(bar_count, frequency, field, ffill)
daily_at_midnight = self.sim_params.data_frequency == 'daily'

This comment has been minimized.

Sign in to view

Copy link
@ehebert

ehebert Jun 30, 2014

Contributor

Small legibility note, and this may be a de gustibus argument and thus ignored, but I find using parens help make the reading of the assignment of a boolean expression a little easier:

daily_at_midnight = (self.sim_params.data_frequency == 'daily')

This comment has been minimized.

Sign in to view

Copy link
@twiecki

twiecki Jun 30, 2014

Author Contributor

Agreed.

history_spec = HistorySpec(bar_count, frequency, field, ffill,
daily_at_midnight=daily_at_midnight)
self.history_specs[history_spec.key_str] = history_spec

@api_method
Expand Down
16 changes: 11 additions & 5 deletions zipline/history/history.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class Frequency(object):
SUPPORTED_FREQUENCIES = frozenset({'1d', '1m'})
MAX_MINUTES = {'m': 1, 'd': 390}

def __init__(self, freq_str):
def __init__(self, freq_str, daily_at_midnight=False):

if freq_str not in self.SUPPORTED_FREQUENCIES:
raise ValueError(
Expand All @@ -56,25 +56,30 @@ def __init__(self, freq_str):
# unit_str - The unit type, e.g. 'd'
self.num, self.unit_str = parse_freq_str(freq_str)

self.daily_at_midnight = daily_at_midnight

def next_window_start(self, previous_window_close):
"""
Get the first minute of the window starting after a window that
finished on @previous_window_close.
"""
if self.unit_str == 'd':
return self.next_day_window_start(previous_window_close)
return self.next_day_window_start(previous_window_close,
self.daily_at_midnight)
elif self.unit_str == 'm':
return self.next_minute_window_start(previous_window_close)

@staticmethod
def next_day_window_start(previous_window_close):
def next_day_window_start(previous_window_close, daily_at_midnight=False):
"""
Get the next day window start after @previous_window_close. This is
defined as the first market open strictly greater than
@previous_window_close.
"""
env = trading.environment
next_open, _ = env.next_open_and_close(previous_window_close)
if daily_at_midnight:
next_open = next_open.replace(hour=0, minute=0, second=0)

This comment has been minimized.

Sign in to view

Copy link
@ehebert

ehebert Jun 30, 2014

Contributor

Elsewhere in the code base we use pd.datetools.normalize_date for the truncation of a date to midnight.

This comment has been minimized.

Sign in to view

Copy link
@twiecki

twiecki Jun 30, 2014

Author Contributor

Ah, that's much better.

This comment has been minimized.

Sign in to view

Copy link
@ehebert

ehebert Jun 30, 2014

Contributor

Also, we may want to further strengthen that they are using separate indexes and do this as:

if daily_at_midnight:
    next_open = env.next_trading_day(previous_window_close)
else:
    next_open, _ = env.next_open_and_close(previous_window_close)
return next_open

@staticmethod
Expand Down Expand Up @@ -229,11 +234,12 @@ def spec_key(cls, bar_count, freq_str, field, ffill):
return "{0}:{1}:{2}:{3}".format(
bar_count, freq_str, field, ffill)

def __init__(self, bar_count, frequency, field, ffill):
def __init__(self, bar_count, frequency, field, ffill,
daily_at_midnight=True):
# Number of bars to look back.
self.bar_count = bar_count
if isinstance(frequency, str):
frequency = Frequency(frequency)
frequency = Frequency(frequency, daily_at_midnight)
# The frequency at which the data is sampled.
self.frequency = frequency
# The field, e.g. 'price', 'volume', etc.
Expand Down

2 comments on commit 5f01e11

@ehebert
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thoughts about using just using the value of 'data_frequency' to trigger the 'at midnight' logic, instead of passing around the daily_at_midnight?

Though I tried a quick pass at passing around data_frequency and the unit_str, maybe making the next window open method a non-static function would help, the logic could look like:

if self.num_str == 'd' and self.data_frequency == 'daily':
    # Get the midnight data here.

Or could add a property of use_midnight which would alias to the above:

@property
def use_midnight(self):
   return (self.num_str == 'd' and self.data_frequency == 'daily')

def next_day_window_start(self):
    env = ...
    if self.use_midnight:
        next_open = # get midnight date here
    else:
       next_open = # get next open as before

@twiecki
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I wondered this as well. The reason I made it configurable is our discussion on what the correct behavior should look like and decided that daily events should be timed at market_close. Thus if someone has data that has the correct dt this wouldn't be necessary. Or if we end up changing it.

Please sign in to comment.