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.

Sign up for GitHub

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

added analyze_backtest which can be turned off. #717

Merged
merged 1 commit into from
Mar 5, 2025
Merged

added analyze_backtest which can be turned off. #717

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions lumibot/example_strategies/classic_60_40.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,8 @@
datasource_class=AlpacaBacktesting,
backtesting_start= datetime(2023, 1, 2),
backtesting_end=datetime(2025, 1, 1),
benchmark_asset=None,
analyze_backtest=False,
parameters=parameters,
show_plot=False,
show_tearsheet=False,
save_tearsheet=False,
show_indicators=False,
save_logfile=False,
show_progress_bar=True,

# AlpacaBacktesting kwargs
Expand Down
6 changes: 1 addition & 5 deletions lumibot/example_strategies/crypto_50_50.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,7 @@
backtesting_start=datetime(2025, 1, 1),
backtesting_end=datetime(2025, 2, 1),
parameters=parameters,
show_plot=False,
show_tearsheet=False,
save_tearsheet=False,
show_indicators=False,
save_logfile=False,
analyze_backtest=False,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Example Doesn't Demonstrate New Feature category Functionality

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

Report a problem with this comment

💬 Chat with Korbit by mentioning @korbit-ai.

show_progress_bar=True,

# AlpacaBacktesting kwargs
Expand Down
8 changes: 8 additions & 0 deletions lumibot/strategies/_strategy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ def __init__(
stats_file=None,
risk_free_rate=None,
benchmark_asset: str | Asset | None = "SPY",
analyze_backtest: bool = True,
backtesting_start=None,
backtesting_end=None,
quote_asset=Asset(symbol="USD", asset_type="forex"),
Expand Down Expand Up @@ -179,6 +180,8 @@ def __init__(
benchmark_asset : Asset or str or None
The asset to use as the benchmark for the strategy. Defaults to "SPY". Strings are converted to
Asset objects with an asset_type="stock". None, means don't benchmark the strategy.
analyze_backtest: bool
Run the backtest_analysis function at the end.
backtesting_start : datetime.datetime
The date and time to start backtesting from. Required for backtesting.
backtesting_end : datetime.datetime
Expand Down Expand Up @@ -332,6 +335,7 @@ def __init__(
self.is_backtesting = self.broker.IS_BACKTESTING_BROKER

self._benchmark_asset = benchmark_asset
self._analyze_backtest = analyze_backtest

# Get the backtesting start and end dates from the broker data source if we are backtesting
if self.is_backtesting:
Expand Down Expand Up @@ -956,6 +960,7 @@ def run_backtest(
name = None,
budget = None,
benchmark_asset: str | Asset | None="SPY",
analyze_backtest: bool = True,
plot_file_html = None,
trades_file = None,
settings_file = None,
Expand Down Expand Up @@ -1017,6 +1022,8 @@ def run_backtest(
benchmark_asset : str or Asset or None
The benchmark asset to use for the backtest to compare to. If it is a string then it will be converted
to a stock Asset object. If it is None, no benchmarking will occur.
analyze_backtest: bool = True
Run the backtest_analysis method on the strategy.
plot_file_html : str
The file to write the plot html to.
trades_file : str
Expand Down Expand Up @@ -1104,6 +1111,7 @@ def run_backtest(
name = self.__name__

self._name = name
self._analyze_backtest = analyze_backtest

# If backtesting_start is None, then check the BACKTESTING_START environment variable
if backtesting_start is None and BACKTESTING_START is not None:
Expand Down
5 changes: 4 additions & 1 deletion lumibot/strategies/strategy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2905,6 +2905,9 @@ def write_backtest_settings(self, settings_file: str):
>>> self.write_backtest_settings("backtest_settings.json")

"""
if not self._analyze_backtest:
return
Comment on lines +2908 to +2909
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing Initialization of analyze_backtest Flag category Functionality

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

Report a problem with this comment

💬 Chat with Korbit by mentioning @korbit-ai.


datasource = self.broker.data_source
auto_adjust = datasource.auto_adjust if hasattr(datasource, "auto_adjust") else False
settings = {
Expand All @@ -2920,7 +2923,7 @@ def write_backtest_settings(self, settings_file: str):
"quote_asset": self.quote_asset,
"benchmark_asset": self._benchmark_asset,
"starting_positions": self.starting_positions,
"parameters": self.parameters,
"parameters": {k: v for k, v in self.parameters.items() if k != 'pandas_data'}
}
os.makedirs(os.path.dirname(settings_file), exist_ok=True)
with open(settings_file, "w") as outfile:
Expand Down
27 changes: 13 additions & 14 deletions lumibot/tools/save_data.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import logging
import os
import sys
from datetime import datetime
from zoneinfo import ZoneInfo

from lumibot.credentials import ALPACA_TEST_CONFIG
from lumibot.backtesting import AlpacaBacktesting
Expand All @@ -10,22 +10,21 @@

if __name__ == "__main__":

tickers = [
"BTC/USD",
"AMZN"
]
start_date = "2021-01-01"
end_date = "2021-01-10"
# timestep = 'day'
timestep = 'hour'
# timestep = 'minute'
datetime_start = datetime(2025, 1, 13)
datetime_end = datetime(2025, 1, 18)
Comment on lines +13 to +14
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Invalid Future Dates for Backtesting category Functionality

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)

Report a problem with this comment

💬 Chat with Korbit by mentioning @korbit-ai.

timestep = 'day'
tzinfo = ZoneInfo("America/New_York")
tickers = "TSLA"
refresh_cache = False
warm_up_trading_days = 0

data_source = AlpacaBacktesting(
datetime_start=datetime_start,
datetime_end=datetime_end,
config=ALPACA_TEST_CONFIG,
tickers=tickers,
start_date=start_date,
end_date=end_date,
timestep=timestep,
config=ALPACA_TEST_CONFIG,
refresh_cache=refresh_cache,
tzinfo=tzinfo,
warm_up_trading_days=warm_up_trading_days
)
26 changes: 13 additions & 13 deletions tests/test_alpaca_backtesting.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ def test_single_stock_day_bars_america_new_york(self):
datasource_class=AlpacaBacktesting,
backtesting_start=backtesting_start,
backtesting_end=backtesting_end,
benchmark_asset=None,
analyze_backtest=False,
parameters={
"asset": Asset('AMZN', asset_type='stock'),
"sleeptime": "1D",
Expand Down Expand Up @@ -117,7 +117,7 @@ def test_get_historical_prices_single_stock_day_bars_america_new_york(self):
datasource_class=AlpacaBacktesting,
backtesting_start=backtesting_start,
backtesting_end=backtesting_end,
benchmark_asset=None,
analyze_backtest=False,
parameters={
"asset": Asset('AMZN', asset_type='stock'),
"sleeptime": "1D",
Expand Down Expand Up @@ -183,7 +183,7 @@ def test_single_stock_minute_bars_america_new_york_regular_hours(self):
backtesting_start=backtesting_start,
backtesting_end=backtesting_end,
minutes_before_closing=0,
benchmark_asset=None,
analyze_backtest=False,
parameters={
"asset": Asset('AMZN', asset_type='stock'),
"sleeptime": "1M",
Expand Down Expand Up @@ -264,7 +264,7 @@ def test_single_stock_minute_bars_america_new_york_extended_hours(self):
backtesting_start=backtesting_start,
backtesting_end=backtesting_end,
minutes_before_closing=0,
benchmark_asset=None,
analyze_backtest=False,
parameters={
"asset": Asset('AMZN', asset_type='stock'),
"sleeptime": "1M",
Expand Down Expand Up @@ -344,7 +344,7 @@ def test_single_stock_30_minute_bars_america_new_york_regular_hours(self):
backtesting_start=backtesting_start,
backtesting_end=backtesting_end,
minutes_before_closing=0,
benchmark_asset=None,
analyze_backtest=False,
parameters={
"asset": Asset('AMZN', asset_type='stock'),
"sleeptime": "30M",
Expand Down Expand Up @@ -428,7 +428,7 @@ def test_single_stock_minute_bars_america_new_york_with_60m_sleeptime(self):
backtesting_start=backtesting_start,
backtesting_end=backtesting_end,
minutes_before_closing=0,
benchmark_asset=None,
analyze_backtest=False,
parameters={
"asset": Asset('AMZN', asset_type='stock'),
"sleeptime": "60M",
Expand Down Expand Up @@ -505,7 +505,7 @@ def test_single_stock_hour_bars_america_new_york(self):
backtesting_start=backtesting_start,
backtesting_end=backtesting_end,
minutes_before_closing=0,
benchmark_asset=None,
analyze_backtest=False,
parameters={
"asset": Asset('AMZN', asset_type='stock'),
"sleeptime": "60M",
Expand Down Expand Up @@ -590,7 +590,7 @@ def test_get_historical_prices_single_stock_hour_bars_america_new_york(self):
datasource_class=AlpacaBacktesting,
backtesting_start=backtesting_start,
backtesting_end=backtesting_end,
benchmark_asset=None,
analyze_backtest=False,
parameters={
"asset": Asset('AMZN', asset_type='stock'),
"sleeptime": "1D",
Expand Down Expand Up @@ -666,7 +666,7 @@ def test_single_crypto_day_bars_america_chicago(self):
backtesting_start=backtesting_start,
backtesting_end=backtesting_end,
minutes_before_closing=0,
benchmark_asset=None,
analyze_backtest=False,
parameters={
"asset": Asset('BTC', asset_type='crypto'),
"sleeptime": "1D",
Expand Down Expand Up @@ -749,7 +749,7 @@ def test_single_crypto_minute_bars_america_chicago(self):
backtesting_start=backtesting_start,
backtesting_end=backtesting_end,
minutes_before_closing=0,
benchmark_asset=None,
analyze_backtest=False,
parameters={
"asset": Asset('BTC', asset_type='crypto'),
"sleeptime": "1M",
Expand Down Expand Up @@ -831,7 +831,7 @@ def test_single_crypto_hour_bars_utc(self):
backtesting_start=backtesting_start,
backtesting_end=backtesting_end,
minutes_before_closing=0,
benchmark_asset=None,
analyze_backtest=False,
parameters={
"asset": Asset('BTC', asset_type='crypto'),
"sleeptime": "60M",
Expand Down Expand Up @@ -915,7 +915,7 @@ def test_single_crypto_hour_bars_america_chicago(self):
backtesting_start=backtesting_start,
backtesting_end=backtesting_end,
minutes_before_closing=0,
benchmark_asset=None,
analyze_backtest=False,
parameters={
"asset": Asset('BTC', asset_type='crypto'),
"sleeptime": "60M",
Expand Down Expand Up @@ -998,7 +998,7 @@ def test_get_historical_prices_single_crypto_hour_bars_america_chicago(self):
backtesting_start=backtesting_start,
backtesting_end=backtesting_end,
minutes_before_closing=0,
benchmark_asset=None,
analyze_backtest=False,
parameters={
"asset": Asset('BTC', asset_type='crypto'),
"sleeptime": "60M",
Expand Down
Loading