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

Perform account validation at the end of the day #1884

Merged
merged 1 commit into from Jul 31, 2017

Conversation

@dmichalowicz
Copy link
Contributor

@dmichalowicz dmichalowicz commented Jul 11, 2017

No description provided.

@coveralls
Copy link

@coveralls coveralls commented Jul 11, 2017

Coverage Status

Coverage increased (+0.0008%) to 87.493% when pulling c1ece56 on validate-account-eod into fb248ef on master.

@dmichalowicz dmichalowicz force-pushed the validate-account-eod branch 2 times, most recently from 75f3083 to 0b238ca Jul 20, 2017
@coveralls
Copy link

@coveralls coveralls commented Jul 20, 2017

Coverage Status

Changes Unknown when pulling 0b238ca on validate-account-eod into ** on master**.

@dmichalowicz dmichalowicz requested a review from richafrank Jul 20, 2017
@coveralls
Copy link

@coveralls coveralls commented Jul 20, 2017

Coverage Status

Coverage increased (+0.0008%) to 87.616% when pulling 0b238ca on validate-account-eod into 42ea84d on master.

@dmichalowicz
Copy link
Contributor Author

@dmichalowicz dmichalowicz commented Jul 28, 2017

@richafrank Updated. I removed the test I added because this behavior is already being tested here: https://github.com/quantopian/zipline/blob/master/tests/test_algorithm.py#L3511

@richafrank
Copy link
Member

@richafrank richafrank commented Jul 28, 2017

I removed the test I added because this behavior is already being tested

Nice - does that mean the existing test didn't care about minutely versus daily?

@dmichalowicz
Copy link
Contributor Author

@dmichalowicz dmichalowicz commented Jul 28, 2017

does that mean the existing test didn't care about minutely versus daily?

Correct, it's simply checking that the appropriate exception is raised when running an algo under the leverage constraint. Do you think this is sufficient?

@coveralls
Copy link

@coveralls coveralls commented Jul 28, 2017

Coverage Status

Coverage decreased (-0.1%) to 87.511% when pulling a3427d3 on validate-account-eod into 42ea84d on master.

@richafrank
Copy link
Member

@richafrank richafrank commented Jul 28, 2017

I suppose we could test to see how far the backtest got. I don't have a strong opinion in this case.

@coveralls
Copy link

@coveralls coveralls commented Jul 28, 2017

Coverage Status

Coverage decreased (-0.5%) to 87.148% when pulling a3427d3 on validate-account-eod into 42ea84d on master.

@dmichalowicz dmichalowicz force-pushed the validate-account-eod branch from a3427d3 to 051b120 Jul 28, 2017
@dmichalowicz
Copy link
Contributor Author

@dmichalowicz dmichalowicz commented Jul 28, 2017

@richafrank I added a time check to the test.

@dmichalowicz dmichalowicz force-pushed the validate-account-eod branch from 051b120 to cfa7600 Jul 28, 2017
@coveralls
Copy link

@coveralls coveralls commented Jul 28, 2017

Coverage Status

Coverage remained the same at 87.511% when pulling cfa7600 on validate-account-eod into 8a886b8 on master.

@dmichalowicz
Copy link
Contributor Author

@dmichalowicz dmichalowicz commented Jul 28, 2017

@richafrank Good to go?

@coveralls
Copy link

@coveralls coveralls commented Jul 28, 2017

Coverage Status

Coverage remained the same at 87.511% when pulling cfa7600 on validate-account-eod into 8a886b8 on master.

@richafrank
Copy link
Member

@richafrank richafrank commented Jul 28, 2017

:shipit:

@dmichalowicz dmichalowicz merged commit ef9c3e9 into master Jul 31, 2017
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@dmichalowicz dmichalowicz deleted the validate-account-eod branch Jul 31, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.