-
Notifications
You must be signed in to change notification settings - Fork 387
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
refactor: move IO responsibility out of Processor trait #134
base: master
Are you sure you want to change the base?
refactor: move IO responsibility out of Processor trait #134
Conversation
Oops, few |
Can you write test cases? I will look into this separately. But since the code is growing and refactored widely, it becomes hard to check if works as expected (the backtesting result is the same as the previous). |
Yes, I think we can start by testing that the |
It looks good. It can be merged after the tests have been completed. |
@nkaz001 added a simple unit test that runs the actual |
De-duplicates the event reading/data management code contained within each processor and delegates it to the `Backtester` to handle.
4b153b8
to
2cff1e5
Compare
IMO, the original implementation needs refactoring to make it more unit testable at the function level. Currently, I am testing it by running the tutorials. Similarly, it would be better to run a testing strategy, which generates a high turnover, with test data and verifying results, such as the final P&L and turnover, for a holistic test. |
I agree. I've started to move handling of event intents out of |
De-duplicates the event reading/data management code contained within each processor and delegates it to the
Backtester
to handle.Standalone version of the changes in #124, but without any of the parallel reader code.