Skip to content
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

sessions_window with high negative count cycles back indices #124

Closed
pavitrakumar78 opened this issue Feb 11, 2020 · 3 comments
Closed

sessions_window with high negative count cycles back indices #124

pavitrakumar78 opened this issue Feb 11, 2020 · 3 comments

Comments

@pavitrakumar78
Copy link
Contributor

pavitrakumar78 commented Feb 11, 2020

Hi,

I've been experimenting with sessions_window and I found that when the count parameter is negative and greater than the total number of days in the trading calendar, the result cycles back and returns the dates between the end date and till the exact overflow value.

Example:

from trading_calendars import get_calendar
market_calendar = get_calendar('XNYS')
end_date = '2019-02-27'
days_rqd = -8500
dates = market_calendar.sessions_window(end_date, days_rqd)
print(dates)

>>DatetimeIndex(['2016-07-12 00:00:00+00:00', '2016-07-13 00:00:00+00:00',
               '2016-07-14 00:00:00+00:00', '2016-07-15 00:00:00+00:00',
               '2016-07-18 00:00:00+00:00', '2016-07-19 00:00:00+00:00',
               '2016-07-20 00:00:00+00:00', '2016-07-21 00:00:00+00:00',
               '2016-07-22 00:00:00+00:00', '2016-07-25 00:00:00+00:00',
               ...
               '2019-02-13 00:00:00+00:00', '2019-02-14 00:00:00+00:00',
               '2019-02-15 00:00:00+00:00', '2019-02-19 00:00:00+00:00',
               '2019-02-20 00:00:00+00:00', '2019-02-21 00:00:00+00:00',
               '2019-02-22 00:00:00+00:00', '2019-02-25 00:00:00+00:00',
               '2019-02-26 00:00:00+00:00', '2019-02-27 00:00:00+00:00'],
              dtype='datetime64[ns, UTC]', length=662, freq='C')

print(len(dates))
>> 662

Upon checking the code for this in trading_calendar.py, start_idx is 7345 for the calendar I've chosen in the above example and end_idx becomes 7345 + (-8500) = -1155 and thus the output will be all_sessions[-1155: 7345 + 1] which is not what I expected. The total length of all_sessions is 7839 - so, converting the negative indices to normal indices, we get all_sessions[7839-1155: 7345 + 1] which is all_sessions[6684: 7345 + 1] which explains why I got 662 dates.

What I expect is all_sessions[0: 7345 + 1] which can be obtained by adding end_idx = max(0, end_idx) right before the indexing on all_sessions (created a pull request for the same - #125). I'm not yet sure if this affects any other functionality, but tl;dr version is, when we query dates where the lookback/negative count is greater than the length of historical dates, it should just return all the available dates till the end date instead of looping back on the indices.

pavitrakumar78 added a commit to pavitrakumar78/trading_calendars that referenced this issue Feb 11, 2020
Made sure that `end_idx` in `sessions_window` method is always positive so that it does not loop-back when indexing on all_sessions when the count/lookback period is greater than the length of total historical dates. More details in quantopian#124.
@gerrymanoim
Copy link
Collaborator

Hey @pavitrakumar78 - thank you for this report/PR!

I very much agree that count should not cycle back when count is large. However, I think my personal preference here is to raise ValueError when end_idx < 0, rather than giving you back fewer than requested elements.

Thoughts @ssanderson @llllllllll ?

@pavitrakumar78
Copy link
Contributor Author

pavitrakumar78 commented Feb 18, 2020

@gerrymanoim
Maybe I was a bit biased here since, in my application, I don't know the exact look-back days, so I relied on the library "supporting" me here by giving me whatever is left/available.

But I was also drawing from how arrays in python work in general. For example, in python doing a[0:10] when a = [0,1,2,3,4] is going to give you [0,1,2,3,4] and not throw an error.

@gerrymanoim
Copy link
Collaborator

Hey @pavitrakumar78 - apologies for the slow reply. I agree with you - this should behave as list slicing does.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants