added analyze_backtest which can be turned off. #717
Conversation
|
Important Required App Permission UpdateNoise Reduction ImprovementsThis update requests write permissions for Commit Statuses in order to send updates directly to your PRs without adding comments that spam notifications. Visit our changelog to learn more. Click here to accept the updated permissions To accept the updated permissions, sufficient privileges are required |
![korbit-ai[bot]](https://avatars.githubusercontent.com/in/322216?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review by Korbit AI
Korbit automatically attempts to detect when you fix issues in new commits.
| Category | Issue | Fix Detected |
|---|---|---|
 |
Invalid Future Dates for Backtesting ▹ view | |
|
Example Doesn't Demonstrate New Feature ▹ view | |
|
Missing Initialization of analyze_backtest Flag ▹ view |
Files scanned
| File Path | Reviewed |
|---|---|
| lumibot/tools/save_data.py | ✅ |
| lumibot/example_strategies/classic_60_40.py | ✅ |
| lumibot/example_strategies/crypto_50_50.py | ✅ |
| lumibot/strategies/_strategy.py | ✅ |
| lumibot/strategies/strategy.py | ✅ |
Explore our documentation to understand the languages and file types we support and the files we ignore.
Need a new review? Comment
/korbit-reviewon this PR and I'll review your latest changes.Korbit Guide: Usage and Customization
Interacting with Korbit
- You can manually ask Korbit to review your PR using the
/korbit-reviewcommand in a comment at the root of your PR.- You can ask Korbit to generate a new PR description using the
/korbit-generate-pr-descriptioncommand in any comment on your PR.- Too many Korbit comments? I can resolve all my comment threads if you use the
/korbit-resolvecommand in any comment on your PR.- Chat with Korbit on issues we post by tagging @korbit-ai in your reply.
- Help train Korbit to improve your reviews by giving a 👍 or 👎 on the comments Korbit posts.
Customizing Korbit
- Check out our docs on how you can make Korbit work best for you and your team.
- Customize Korbit for your organization through the Korbit Console.
Current Korbit Configuration
General Settings
Setting Value Review Schedule Automatic excluding drafts Max Issue Count 10 Automatic PR Descriptions ✅ Issue Categories
Category Enabled Documentation ✅ Logging ✅ Error Handling ✅ Readability ✅ Design ✅ Performance ✅ Security ✅ Functionality ✅ Feedback and Support
Note
Korbit Pro is free for open source projects 🎉
Looking to add Korbit to your team? Get started with a free 2 week trial here
| datetime_start = datetime(2025, 1, 13) | ||
| datetime_end = datetime(2025, 1, 18) |
There was a problem hiding this comment.
Invalid Future Dates for Backtesting 
Tell me more
What is the issue?
The backtesting dates are set in the future (2025), which will result in no historical data being available for analysis.
Why this matters
Alpaca will not be able to provide historical price data for future dates, causing the backtesting to fail or return empty results.
Suggested change ∙ Feature Preview
Change the dates to a past time period where historical data is available. For example:
datetime_start = datetime(2023, 1, 13)
datetime_end = datetime(2023, 1, 18)
💬 Chat with Korbit by mentioning @korbit-ai.
| save_tearsheet=False, | ||
| show_indicators=False, | ||
| save_logfile=False, | ||
| analyze_backtest=False, |
There was a problem hiding this comment.
Example Doesn't Demonstrate New Feature
Tell me more
What is the issue?
Setting analyze_backtest to False contradicts the developer's intent to add flexibility for backtest analysis.
Why this matters
Hard-coding this value to False in the example prevents users from seeing how to properly utilize the new analyze_backtest feature.
Suggested change ∙ Feature Preview
Either make the analyze_backtest parameter configurable or set it to True to demonstrate the feature:
analyze_backtest=True # or make it a parameter💬 Chat with Korbit by mentioning @korbit-ai.
| if not self._analyze_backtest: | ||
| return |
There was a problem hiding this comment.
Missing Initialization of analyze_backtest Flag
Tell me more
What is the issue?
The code adds a check for _analyze_backtest but there is no robust initialization or configuration of this flag. The flag could be undefined or uninitialized.
Why this matters
Without proper initialization, accessing _analyze_backtest could raise an AttributeError, causing the backtest functionality to fail unexpectedly.
Suggested change ∙ Feature Preview
Add proper initialization of the _analyze_backtest flag in the strategy's initialization code:
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self._analyze_backtest = kwargs.get('analyze_backtest', True) # Default to True for backward compatibility💬 Chat with Korbit by mentioning @korbit-ai.
Description by Korbit AI
What change is being made?
Introduce an
analyze_backtestparameter across various strategy configurations to control if backtest analysis should be performed, and update related code and tests accordingly.Why are these changes being made?
This change provides the ability to toggle backtest analysis on and off, offering more flexibility for users who might not need or want to perform analysis for every backtest execution. By default,
analyze_backtestis enabled but can be disabled where it's unnecessary, improving efficiency and customization in backtesting workflows.