-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WIP: (DO NOT MERGE) New ExchangeCalendar API #556
Conversation
51c92e3
to
6515aae
Compare
6515aae
to
3a92e68
Compare
3a92e68
to
2424a90
Compare
ACK'd will read through by end of day. |
Sounds great but imo there lacks a connection between the calendar and other marketplace specific variables such as the benchmark, commission and asset metadata. I think we need another level of abstraction, maybe an
The benchmark loader is a massive barrier (as big as the calendar problems) for anyone like myself looking to develop Zipline for other marketplaces. Is is possible to make this more user-friendly? It would also be great to see a connect between @jfkirk's current work on the Asset class incorporated, for example linking product metadata to an Exchange. |
On another note, it would be great to be able to have some flexibility with |
Warren's feedback about wrapping this all together in exchange-level information makes sense to me - it will be valuable as more Assets in more exchanges are added to our system. @warren-oneill The Asset object currently has a field for its exchange, but we don't have it set anywhere. Are you looking to perform one simulation with Assets from many different exchanges and, therefore, many different trading hours? This is on my roadmap, but was not my immediate plan. |
@jfkirk far down the line we would like to trade the same asset on different exchanges but what I'd like in the short term is to just have to define the exchange and then have the assets automatically linked/initialized but presumably our list of assets is a lot shorter than your list of assets. |
# curves would be the day before the date considered to be | ||
# 'today'. | ||
self.treasury_curves = tr_c[tr_c.index <= max_date] | ||
self.treasury_curves = self.treasury_curves.query("index <= @max_date") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TIL about the @
symbol in the query syntax. Nice.
@ssanderson 👍 on using the Holiday syntax, as well as 👍 on instantiating with-in the trading environment. So looks good to move forward with. @warren-oneill @jfkirk I could see where there would be an environment with that tighter exchange binding, and perhaps multiple exchanges and calendars. This design doesn't solve that problem now, but I think is a good step along the way. |
I'm on board with this. I suppose the question is whether it's worth having the top-level exchange class that's separate from the calendar class, or whether we should add "default_exchange" or somesuch to A sketch of what an
@warren-oneill @jfkirk what other kinds of metadata would you want to be associated with an exchange? |
@ssanderson I would def prefer to have an exchange class in the vain that you illustrated above. Personally I would also like to see a default commission and an AssetMetaData class as part of the Exchange class. |
hey @ssanderson is this on ice? |
@warren-oneill been superceded as an immediate need by #584, though I'm trying to allocate ~20% time to Zipline community needs. Are there features here that would be particularly important to the work you're doing with Zipline? |
@ssanderson it's not a barrier anymore now that I have everything setup (I made my own exchange class locally) but for would be a big help for new users to have everything in one place. What's still unclear to me what is the correct order of initialization of env, asset_finder and source but perhaps the order doesn't matter. @jfkirk? |
FYI here's a snippet of my exchange base class
any functions not defined are abstractmethods and are defined in the subclasses and |
@warren-oneill thanks for the snippet. I'm hoping to get back to this some time soon. |
Closing this in deference to @jfkirk's current work on bringing this in for real. |
@ssanderson @jfkirk is there a branch or PR? |
WIP for ExchangeCalendar redesign. Still in the process of fixing up the tests and porting over the non NYSE calendars, but I think something like this API is what we want.
The essential thought is that, rather than passing an entire module (which among other things, causes us to do substantial computation at import time, slows down our tests, and makes simulation startup noticeably slower), you now pass a subclass of
ExchangeCalendar
toTradingEnvironment
. The environment then instantiates the calendar, which provides three public attributes:day
(an instance ofpandas.CustomBusinessDay
)schedule
(DataFrame
with the same structure as the oldopen_and_closes
)native_timezone
(a timezone)I haven't yet re-implemented any of the date logic that's done manually in
TradingEnvironment
, but much of it could be thrown out in favor of using the new CustomBusinessDay. For example,next_trading_day(dt)
can be written as just(dt + calendar.day)
, and it's also possible to compute ranges of trading days by passingcalendar.day
asfreq
topd.date_range
.Implementing a new
ExchangeCalendar
can be done by subclassing and implementing the following abstract properties (in practice I expect most of them to be implemented as class-level attributes: note that this doesn't have the same issue of doing work at module scope as the old implementation, because instantiating anAbstractHolidayCalendar
doesn't do any work until you callcalendar.dates
to actually extract its dates):native_timezone
: A timezone in which to interpretopen_time
andclose_time
.open_time
: Adatetime.time
representing the first tradable minute of the exchange on a normal day).close_time
: Adatetime.time
representing the final tradable minute of the exchange on a normal day.holidays_calendar
: A pandasAbstractHolidayCalendar
instance defining days on which the exchange is not open.special_opens_calendars
: A (possibly empty) iterable of pairs of (datetime.time
,AbstractHolidayCalendar
). Each calendar is used to define days on which the exchange regularly opens at a time other than the standard open. Multiple calendars are allowed to account for the possibility that the exchange opens at different nonstandard times under different circumstances.special_closes_calendars
: Same structure asspecial_opens_calendars
, but defines nonstandard closing times. For the main NYSE calendar, this is used to define the various regular early closes.holidays_adhoc
: An iterable of dates defining days on which the exchange was closed due to historical accident (e.g. the hurricane sandy closings for NYSE, or the world cup closings for BMF).special_opens_adhoc
: A (possibly empty) iterable of pairs of (datetime.time
, list ofTimestamp
). Each entry defines a list of additional dates on which the exchange opened at a nonstandard time due to historical accident.special_closes_adhoc
: Same structure asspecial_opens_adhoc
, but defines anomalous closes.I've already re-implemented the NYSE calendar in terms of this interface, and I think it's significantly cleaner than the old version. It's particularly nice to be able to use pandas' built-in holiday functionality to write something like:
and not have to do all the special-casing logic for what happens when Christmas falls on a weekend. Implementing Black Friday (the day after the Thanksgiving, i.e. the 4th thursday in November) as
is also quite nice.
@warren-oneill, @jfkirk, @ehebert, @twiecki I'd like to get your guys' feedback on the API presented here, both for creating new calendars and for consuming them in
TradingEnvironment
while I'm wrangling the remaining tests.