BUG/ENH: Allow users to switch between calendars #1800

freddiev4 · 2017-05-21T18:54:28Z

You previously could not switch between different calendars because the trading_calendar parameter was not being passed into run_algo or TradingAlgorithm. Once merged, you should be able to pass which calendar you'd like to use via the CLI using --trading-calendar.

e.g.
zipline run -f algo.py --start 2012-01-01 --end 2013-01-01 --trading-calendar LSE

Caveats:

TradingAlgorithm and SimulationParameters both take a trading_calendar object as arguments. Rather than going down the rabbit hole of changing up the implementation of both of those classes, I decided to just allow users to first have the capability of using different calendars, and set aside the re-implementation issue as a separate project at a later point. TradingEnvironment also takes a trading_calendar object
You need data that fits the dates associated with the trading calendar you want to use; e.g., if you choose LSE, you need a bundle that has data for every date in the LSE Calendar; the default bundle does not have that (dates such as January 2nd, and July 4th are missing).

freddiev4 · 2017-09-14T06:47:53Z

zipline/algorithm.py

+            self.trading_calendar = get_calendar("NYSE")
+        else:
+            self.trading_calendar = get_calendar(calendar)
+


I'm pretty sure that if I'm using kwargs.pop() above that I can remove the if statement and just assign self.trading_calendar to get_calendar(calendar)

Yup, you've encapsulated the logic of the conditional statement in the pop: if trading_calendar is not in the kwargs then use "NYSE".

coveralls · 2017-09-27T23:07:25Z

Coverage decreased (-0.01%) to 87.189% when pulling 86e73a8 on use-different-calendars into 64af3d3 on master.

coveralls · 2017-09-27T23:43:13Z

Coverage decreased (-0.01%) to 87.189% when pulling f72f584 on use-different-calendars into 64af3d3 on master.

coveralls · 2017-09-28T00:13:19Z

Coverage decreased (-0.01%) to 87.189% when pulling fdb50eb on use-different-calendars into fbdfaf8 on master.

coveralls · 2017-09-28T00:13:19Z

Coverage decreased (-0.01%) to 87.189% when pulling fdb50eb on use-different-calendars into fbdfaf8 on master.

coveralls · 2017-09-28T00:13:20Z

Coverage decreased (-0.01%) to 87.189% when pulling fdb50eb on use-different-calendars into fbdfaf8 on master.

freddiev4 · 2017-09-28T01:27:16Z

@yankees714 could you take a look at this? I was wondering if/where I should have more test coverage now that we're adding/fixing this.

yankees714

This is a great change! Had some questions and suggestions for you.

Regarding tests, from what I can tell we don't have much for the CLI or run_algorithm. I do see one call to run_algorithm, so we at least get some coverage there of threading the new arg.

yankees714 · 2017-09-28T13:39:16Z

zipline/__main__.py

+    '--trading-calendar',
+    metavar='TRADING-CALENDAR',
+    default='NYSE',
+    help="The calendar you want to use e.g. LSE. NYSE is the default."


Thoughts on making this a choice option? http://click.pocoo.org/5/options/#choice-options

You could use the keys of _default_calendar_factories in zipline.utils.calendars.calendar_utils to populate it

TIL. Going to make that change right now

yankees714 · 2017-09-28T13:42:03Z

zipline/utils/factory.py

-def create_simulation_parameters(year=2006, start=None, end=None,
+def create_simulation_parameters(year=2006,
+                                 start=None,
+                                 end=None,


yankees714 · 2017-09-28T13:51:04Z

zipline/testing/fixtures.py

@@ -598,6 +598,8 @@ class WithSimParams(WithTradingEnvironment):
    SIM_PARAMS_START = alias('START_DATE')
    SIM_PARAMS_END = alias('END_DATE')

+    SIM_PARAMS_CALENDAR = get_calendar('NYSE')


What's this change for?

I noticed that we were only using cls.trading_calendar for testing; figured I'd switch it to just use NYSE. But now that you've pointed it out, we'd probably not want this change so we can test across whatever calendar is passed in

Ah I also saw test failures because the default trading_calendar is None:

e.g.

====================================================================== FAIL: test_schedule_function_time_rule_positionally_misplaced (tests.test_algorithm.TestAlgoScript) ---------------------------------------------------------------------- Traceback (most recent call last): File "/Users/freddiev4/Documents/quantopian/zipline/tests/test_algorithm.py", line 2272, in test_schedule_function_time_rule_positionally_misplaced data_frequency='minute' File "/Users/freddiev4/Documents/quantopian/zipline/zipline/utils/factory.py", line 62, in create_simulation_parameters trading_calendar=trading_calendar, File "/Users/freddiev4/Documents/quantopian/zipline/zipline/finance/trading.py", line 141, in __init__ "Must pass in trading calendar!" AssertionError: Must pass in trading calendar!

I'm not sure that failure is related, since that test is calling create_simulation_parameters directly, not using the self.sim_params attribute provided by the fixture

yankees714 · 2017-09-28T14:17:06Z

zipline/utils/run_algo.py

@@ -155,15 +156,20 @@ def choose_loader(column):
        env = TradingEnvironment(environ=environ)
        choose_loader = None

+    if not trading_calendar:
+        trading_calendar = get_calendar('NYSE')


Looks like we're already doing this in create_simulation_parameters here, so I'm thinking we don't need handling at this level?

Both TradingAlgorithm and SimulationParameters take a TradingCalendar object so I added those lines to get the calendar object specifically for TradingAlgorithm, IIRC (but thinking about it now, it doesn't make sense)

If anything, I'm thinking I could just remove the handling from create_simulation_parameters, because I'm just using the same object here for both trading algo and simparams.

Thoughts?

Ah, makes sense that we need to pass an actual TradingCalendar when constructing the TradingAlgorithm. Seems like we're using create_simulation_parameters a bunch of places in the tests where we don't pass a calendar, so probably fine to leave this as-is

yankees714 · 2017-09-28T14:44:21Z

etc/requirements_dev.txt

@@ -60,3 +60,6 @@ piprot==0.9.6

 # For mocking out requests fetches
 responses==0.4.0
+
+# Debugging
+pdbpp==0.9.1


Should we pin pdbpp's dependencies as well?

Sure I think that's fair to do

You previously could not switch between different calendars because the `trading_calendar` parameter was not being passed into run_algo or `TradingAlgorithm`. You now can pass which calendar you'd like to use via the CLI using `--trading-calendar`.

coveralls · 2017-10-03T23:40:24Z

Coverage decreased (-0.02%) to 87.193% when pulling 26b77ec on use-different-calendars into 5421ec0 on master.

coveralls · 2017-10-03T23:40:24Z

Coverage decreased (-0.02%) to 87.193% when pulling 26b77ec on use-different-calendars into 5421ec0 on master.

freddiev4 · 2017-10-03T23:41:53Z

@yankees714 I think this should be good for a 2nd review

yankees714

LGTM! Just had one more style suggestion

yankees714 · 2017-10-04T14:10:27Z

zipline/__main__.py

 from zipline.data import bundles as bundles_module
+from zipline.utils.calendars.calendar_utils import (
+    get_calendar,
+    _default_calendar_factories


We probably don't want to keep the leading _ if we're importing this here.

Better yet, what would you think of including a line in calendar_utils like

default_calendar_names = _default_calendar_factories.keys()

and importing that instead?

Sounds good to me. I also wrapped it with sorted()

coveralls · 2017-10-04T19:07:13Z

Coverage decreased (-0.006%) to 87.202% when pulling 5f49dc8 on use-different-calendars into 5421ec0 on master.

coveralls · 2017-10-04T19:07:13Z

Coverage decreased (-0.006%) to 87.202% when pulling 5f49dc8 on use-different-calendars into 5421ec0 on master.

vonpupp · 2017-11-15T20:51:05Z

Hi @freddiev4 ,

I am new to zipline. I am trying to use a 24/7 calendar, so I am trying to use the feature of this PR already to do it in a proper way. I am using zipline installed via pip directly from github's egg (`pip install git+git://github.com/quantopian/zipline')

Unfortunately I am not able to use this feature properly, here is what I am doing:

I edited the extensions.py as follows:

# A bunch of imports removed for the sake of clarity

class TwentyFourSevenCalendar(TradingCalendar):
    """
    Exchange calendar for 24/7 trading.

    Open Time: 12am, UTC
    Close Time: 11:59pm, UTC

    """
    @property
    def name(self):
        return "TWENTYFOURSEVEN"

    @property
    def tz(self):
        return timezone("UTC")

    @property
    def open_time(self):
        return time(0, 0)

    @property
    def close_time(self):
        return time(23,59)

    @lazyval
    def day(self):
        return CustomBusinessDay(
            weekmask='Mon Tue Wed Thu Fri Sat Sun',
        )

start_session = pd.Timestamp('2014-01-07', tz='utc')
end_session = pd.Timestamp('2017-11-13', tz='utc')
assets = ['BTC_USD']

register_calendar(
    'TWENTYFOURSEVEN',
    TwentyFourSevenCalendar(
        start=start_session,
        end=end_session
    )
)
register(
    'custom-csvdir-bundle',
    csvdir_equities(["daily"], '/home/av/repos/ziplinetest/csvdir'),
    calendar_name='TWENTYFOURSEVEN',
    start_session=start_session,
    end_session=end_session
)

As you can see I am using csvdir bundle which has recently been merged. The ingestion process seems to work fine (no errors and I also verified the directory and the sqlite files exist, so I assume the ingestion is ok). I conclude therefore that the 24/7 calendar works properly during ingestion.

When I try to execute with:

zipline run -f csvdir_example.py \
    -b custom-csvdir-bundle \
    --start 2016-1-1 \
    --end 2017-1-1 \
    --data-frequency daily \
    --trading-calendar TWENTYFOURSEVEN \
    --capital-base 100000 \
    -o csvdir_example.pickle

I get:

Error: Invalid value for "--trading-calendar": invalid choice: TWENTYFOURSEVEN. (choose from BMF, CFE, CME, ICE, LSE, NYSE, TSX, us_futures)

Could you please help me?

Thanks a lot.

richafrank · 2017-11-20T18:48:26Z

Thanks @vonpupp - I opened a dedicated issue to track this bug: #2018

freddiev4 force-pushed the use-different-calendars branch from bc60606 to 046c72f Compare May 21, 2017 21:02

freddiev4 added the PyCon2017 Sprint label May 22, 2017

freddiev4 mentioned this pull request May 26, 2017

Trading Calendar API #1774

Closed

freddiev4 changed the title ~~BUG: Allow users to switch between calendars~~ BUG/ENH: Allow users to switch between calendars May 26, 2017

freddiev4 force-pushed the use-different-calendars branch from 046c72f to 475c25a Compare June 5, 2017 18:09

freddiev4 mentioned this pull request Sep 13, 2017

Add Indian Stock Exchanges Calendar #1923

Closed

freddiev4 force-pushed the use-different-calendars branch from 475c25a to 63e11d4 Compare September 14, 2017 06:46

freddiev4 commented Sep 14, 2017

View reviewed changes

freddiev4 force-pushed the use-different-calendars branch from 63e11d4 to 3146de4 Compare September 25, 2017 14:40

freddiev4 force-pushed the use-different-calendars branch from f72f584 to fdb50eb Compare September 27, 2017 23:48

freddiev4 added Calendar Bug and removed PyCon2017 Sprint labels Sep 28, 2017

yankees714 self-requested a review September 28, 2017 13:31

yankees714 reviewed Sep 28, 2017

View reviewed changes

buinvest and others added 2 commits October 3, 2017 15:06

DEV/MAINT: Add pdb++ and change fixtures variable

eb8ddbe

freddiev4 force-pushed the use-different-calendars branch from fdb50eb to eb8ddbe Compare October 3, 2017 21:57

Freddie Vargus added 3 commits October 3, 2017 18:23

STY: Missing import

abb8760

MAINT: Extra file

56dd6e4

MAINT: Fix factory

26b77ec

yankees714 approved these changes Oct 4, 2017

View reviewed changes

STY: Better naming for --trading-calendar options

5f49dc8

freddiev4 merged commit 7cd6f6a into master Oct 5, 2017

freddiev4 deleted the use-different-calendars branch October 5, 2017 14:08

freddiev4 mentioned this pull request Oct 21, 2017

ENH: Implement csvdir bundle #1860

Merged

richafrank mentioned this pull request Nov 20, 2017

Error running zipline backtest with custom trading calendar #2018

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG/ENH: Allow users to switch between calendars #1800

BUG/ENH: Allow users to switch between calendars #1800

freddiev4 commented May 21, 2017 •

edited

freddiev4 Sep 14, 2017

richafrank Sep 14, 2017

coveralls commented Sep 27, 2017 •

edited

coveralls commented Sep 27, 2017 •

edited

coveralls commented Sep 28, 2017

coveralls commented Sep 28, 2017

coveralls commented Sep 28, 2017

freddiev4 commented Sep 28, 2017

yankees714 left a comment

yankees714 Sep 28, 2017

freddiev4 Oct 3, 2017

yankees714 Sep 28, 2017

yankees714 Sep 28, 2017

freddiev4 Oct 3, 2017

freddiev4 Oct 3, 2017

yankees714 Oct 3, 2017

yankees714 Sep 28, 2017

freddiev4 Oct 3, 2017 •

edited

yankees714 Oct 3, 2017

yankees714 Sep 28, 2017

freddiev4 Oct 3, 2017

coveralls commented Oct 3, 2017

coveralls commented Oct 3, 2017 •

edited

freddiev4 commented Oct 3, 2017

yankees714 left a comment

yankees714 Oct 4, 2017

freddiev4 Oct 4, 2017

coveralls commented Oct 4, 2017

coveralls commented Oct 4, 2017 •

edited

vonpupp commented Nov 15, 2017 •

edited

richafrank commented Nov 20, 2017

BUG/ENH: Allow users to switch between calendars #1800

BUG/ENH: Allow users to switch between calendars #1800

Conversation

freddiev4 commented May 21, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Sep 27, 2017 • edited

coveralls commented Sep 27, 2017 • edited

coveralls commented Sep 28, 2017

coveralls commented Sep 28, 2017

coveralls commented Sep 28, 2017

freddiev4 commented Sep 28, 2017

yankees714 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

freddiev4 Oct 3, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Oct 3, 2017

coveralls commented Oct 3, 2017 • edited

freddiev4 commented Oct 3, 2017

yankees714 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Oct 4, 2017

coveralls commented Oct 4, 2017 • edited

vonpupp commented Nov 15, 2017 • edited

richafrank commented Nov 20, 2017

freddiev4 commented May 21, 2017 •

edited

coveralls commented Sep 27, 2017 •

edited

coveralls commented Sep 27, 2017 •

edited

freddiev4 Oct 3, 2017 •

edited

coveralls commented Oct 3, 2017 •

edited

coveralls commented Oct 4, 2017 •

edited

vonpupp commented Nov 15, 2017 •

edited