-
Notifications
You must be signed in to change notification settings - Fork 7
EMA + TWAP backtest #2
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
base: main
Are you sure you want to change the base?
EMA + TWAP backtest #2
Conversation
WalkthroughAdds Pandas and nautilus-trader deps; introduces a Hyperliquid data fetcher that synthesizes trade CSVs, an EMA+TWAP backtest CLI wired to Nautilus Trader, integration tests exercising CSV → TradeTickDataWrangler → BacktestEngine, and README/backtesting docs updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CLI as Backtest CLI
participant Fetcher as Hyperliquid Fetcher
participant SDK as Hyperliquid SDK
participant Wrangler as TradeTickDataWrangler
participant Engine as BacktestEngine
participant Venue as Hyperliquid Venue
participant Strategy as EMACrossTWAP
participant TWAP as TWAPExecAlgorithm
User->>CLI: run script (args: symbol, --refresh-data?)
alt refresh-data
CLI->>Fetcher: fetch_trades_to_csv(symbol, interval, minutes, testnet, out, maxPerCandle)
Fetcher->>SDK: request candles
SDK-->>Fetcher: candles
Fetcher-->>CLI: CSV path
end
CLI->>CLI: load CSV -> DataFrame
CLI->>Wrangler: convert trades -> ticks
Wrangler-->>Engine: feed ticks
CLI->>Venue: register venue & instrument
CLI->>Engine: attach Strategy and TWAP
User->>Engine: start
Engine->>Strategy: deliver ticks
Strategy->>TWAP: issue TWAP execution requests
TWAP->>Venue: execute child orders
Venue-->>Engine: fills & updates
Engine-->>User: reports (accounts, fills, positions)
User->>Engine: reset & dispose
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pyproject.toml (1)
6-6
: Adjust Python requirement or pandas dependency for Python 3.13
NautilusTrader 1.219.0 supports CPython 3.11–3.13, but pandas only added 3.13 support in 2.2.3. Apply one of:Option A (relax Python floor):
-requires-python = ">=3.13" +requires-python = ">=3.11"Option B (bump pandas):
- "pandas>=2.2.0", + "pandas>=2.2.3",
🧹 Nitpick comments (10)
src/backtesting/README.md (1)
1-4
: Document available CLI flags to reduce confusion.Add examples for --mainnet, --interval, and --max-trades-per-candle so users can reproduce the chart with the same knobs.
Suggested addition:
- Example (mainnet): uv run --python 3.13 python src/backtesting/hyperliquid_btc_ema_twap_backtest.py --refresh-data --minutes 720 --mainnet
- Example (custom interval and density): ... --interval 5m --max-trades-per-candle 20
src/backtesting/hyperliquid_data_fetcher.py (4)
1-1
: Shebang without executable bit (EXE001).Either make the file executable in VCS or drop the shebang.
Apply this diff to drop the shebang (script is invoked via
python
anyway):-#!/usr/bin/env python3
71-79
: Ruff TRY003: long exception messages and noisy output.Messages are long and include dynamic lists; okay for CLI, but violates TRY003. Consider shorter messages or a custom exception.
Example:
- available = ", ".join(sorted(info.name_to_coin.keys())) - raise ValueError(f"Unknown Hyperliquid symbol '{symbol}'. Available: {available}") + raise ValueError(f"Unknown symbol: {symbol}")
111-118
: Avoid generating zero-quantity trades due to formatting; write numeric types.Rounding to 6 d.p. can zero-out tiny quantities, producing invalid ticks downstream. Write numeric values and let readers handle formatting.
Apply this diff:
- rows.append( - [ - timestamp, - trade_id, - f"{price:.6f}", - f"{quantity:.6f}", - "True" if buyer_maker else "False", - ] - ) + rows.append([timestamp, trade_id, price, quantity, buyer_maker])If you want strings, add a post-format clamp to skip zero after rounding.
Also applies to: 119-126
155-185
: Nice CLI; small nit: align defaults with README.Defaults mirror the backtest script. Consider adding an explicit
--testnet/--mainnet
mutually exclusive group for clearer UX.src/backtesting/hyperliquid_btc_ema_twap_backtest.py (5)
1-1
: Shebang without executable bit (EXE001).Either drop the shebang or mark the file executable.
-#!/usr/bin/env python3
50-52
: Local import is fine for script execution; consider package-relative import if packaging later.Running via
python src/backtesting/...py
works. If you convert to a package, switch to a relative import.-from hyperliquid_data_fetcher import DEFAULT_INTERVAL -from hyperliquid_data_fetcher import fetch_trades_to_csv +# from .hyperliquid_data_fetcher import DEFAULT_INTERVAL, fetch_trades_to_csv
122-129
: Data loading is sound; minor dtype nit.You could pass dtype={"price": "float64", "quantity": "float64"} to read_csv to avoid a second cast.
- frame = pd.read_csv(path) + frame = pd.read_csv(path, dtype={"price": "float64", "quantity": "float64"})
148-153
: Ensure engine cleanup even on exceptions.Wrap run in try/finally so reset/dispose always execute.
- engine = BacktestEngine(config=config) + engine = BacktestEngine(config=config) @@ - engine.run() - - with pd.option_context("display.max_rows", 100, "display.max_columns", None, "display.width", 300): - print(engine.trader.generate_account_report(HYPERLIQUID_VENUE)) - print(engine.trader.generate_order_fills_report()) - print(engine.trader.generate_positions_report()) - - engine.reset() - engine.dispose() + try: + engine.run() + with pd.option_context("display.max_rows", 100, "display.max_columns", None, "display.width", 300): + print(engine.trader.generate_account_report(HYPERLIQUID_VENUE)) + print(engine.trader.generate_order_fills_report()) + print(engine.trader.generate_positions_report()) + finally: + engine.reset() + engine.dispose()Also applies to: 196-197
187-187
: Avoid arbitrary sleep before run.This should be unnecessary; remove unless there’s a known race.
- time.sleep(0.1)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lock
is excluded by!**/*.lock
📒 Files selected for processing (4)
pyproject.toml
(1 hunks)src/backtesting/README.md
(1 hunks)src/backtesting/hyperliquid_btc_ema_twap_backtest.py
(1 hunks)src/backtesting/hyperliquid_data_fetcher.py
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/backtesting/hyperliquid_data_fetcher.py (1)
src/backtesting/hyperliquid_btc_ema_twap_backtest.py (2)
parse_args
(85-119)main
(132-197)
src/backtesting/hyperliquid_btc_ema_twap_backtest.py (1)
src/backtesting/hyperliquid_data_fetcher.py (2)
fetch_trades_to_csv
(131-152)main
(188-198)
🪛 Ruff (0.13.1)
src/backtesting/hyperliquid_data_fetcher.py
1-1: Shebang is present but file is not executable
(EXE001)
73-73: Avoid specifying long messages outside the exception class
(TRY003)
77-79: Avoid specifying long messages outside the exception class
(TRY003)
src/backtesting/hyperliquid_btc_ema_twap_backtest.py
1-1: Shebang is present but file is not executable
(EXE001)
🔇 Additional comments (5)
src/backtesting/hyperliquid_data_fetcher.py (3)
147-151
: CSV writer can handle mixed types; current approach is fine after above change.Header and bulk write look good. Ensure parent dir exists (already done).
188-199
: Good: CLI entry point prints destination path.This is helpful feedback for users running ad-hoc.
65-76
: No SDK compatibility issues: Info API is stable
Info.candles_snapshot(self, coin: str, interval: str, start_time: int, end_time: int) → list
and thename_to_coin
mapping have been present and unchanged since the initial hyperliquid-python-sdk release; no version bump required.src/backtesting/hyperliquid_btc_ema_twap_backtest.py (2)
59-83
: Instrument definition looks consistent with the wrangler and venue.Precisions/increments and fees are reasonable for a demo backtest.
171-179
: Strategy params: sensible demo defaults.Good starting config for a quick demonstration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
README.md
(1 hunks)src/backtesting/hyperliquid_btc_ema_twap_backtest.py
(1 hunks)tests/conftest.py
(1 hunks)tests/integration/test_backtesting_nautilus.py
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- README.md
- tests/conftest.py
🧰 Additional context used
🧬 Code graph analysis (2)
tests/integration/test_backtesting_nautilus.py (2)
src/backtesting/hyperliquid_data_fetcher.py (1)
fetch_trades_to_csv
(131-152)src/backtesting/hyperliquid_btc_ema_twap_backtest.py (2)
load_trade_data
(126-133)hyperliquid_btcusdc_instrument
(63-86)
src/backtesting/hyperliquid_btc_ema_twap_backtest.py (1)
src/backtesting/hyperliquid_data_fetcher.py (2)
fetch_trades_to_csv
(131-152)main
(188-198)
🪛 Ruff (0.13.1)
tests/integration/test_backtesting_nautilus.py
51-51: Unused method argument: symbol
(ARG002)
51-51: Unused method argument: interval
(ARG002)
51-51: Unused method argument: start_ms
(ARG002)
51-51: Unused method argument: end_ms
(ARG002)
67-67: Unused method argument: event
(ARG002)
src/backtesting/hyperliquid_btc_ema_twap_backtest.py
1-1: Shebang is present but file is not executable
(EXE001)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (3)
tests/integration/test_backtesting_nautilus.py (3)
51-52
: Silence lint: mark unused params in fake API.Rename to underscored args to avoid ARG002 without changing behavior.
- def candles_snapshot(self, symbol: str, interval: str, start_ms: int, end_ms: int) -> list[dict]: + def candles_snapshot(self, _symbol: str, _interval: str, _start_ms: int, _end_ms: int) -> list[dict]: return candles
67-71
: Silence lint: unused event param.Underscore the argument.
- def on_trade_tick(self, event) -> None: + def on_trade_tick(self, _event) -> None: self.received += 1
153-159
: Remove duplicate engine shutdown calls.
reset()
/dispose()
are called twice; keep a single, clean shutdown.assert strategy.received == 30 engine.stop() - engine.reset() - engine.dispose() - engine.reset() - engine.dispose() + engine.reset() + engine.dispose()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/backtesting/hyperliquid_btc_ema_twap_backtest.py
(1 hunks)tests/integration/test_backtesting_nautilus.py
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/backtesting/hyperliquid_btc_ema_twap_backtest.py (1)
src/backtesting/hyperliquid_data_fetcher.py (2)
fetch_trades_to_csv
(131-152)main
(188-198)
tests/integration/test_backtesting_nautilus.py (2)
src/backtesting/hyperliquid_data_fetcher.py (1)
fetch_trades_to_csv
(131-152)src/backtesting/hyperliquid_btc_ema_twap_backtest.py (2)
load_trade_data
(126-133)hyperliquid_btcusdc_instrument
(63-86)
🪛 Ruff (0.13.1)
src/backtesting/hyperliquid_btc_ema_twap_backtest.py
1-1: Shebang is present but file is not executable
(EXE001)
146-151: Positional argument cannot follow keyword argument
(invalid-syntax)
152-152: Expected ',', found name
(invalid-syntax)
153-153: Expected 'else', found ':'
(invalid-syntax)
154-154: Positional argument cannot follow keyword argument
(invalid-syntax)
154-155: Expected ')', found newline
(invalid-syntax)
155-155: unindent does not match any outer indentation level
(invalid-syntax)
tests/integration/test_backtesting_nautilus.py
51-51: Unused method argument: symbol
(ARG002)
51-51: Unused method argument: interval
(ARG002)
51-51: Unused method argument: start_ms
(ARG002)
51-51: Unused method argument: end_ms
(ARG002)
67-67: Unused method argument: event
(ARG002)
🔇 Additional comments (1)
src/backtesting/hyperliquid_btc_ema_twap_backtest.py (1)
118-124
: Validate--max-trades-per-candle
on parse (fail fast, no silent coercion).Add a post-parse check to enforce positive values, then pass through unchanged.
- return parser.parse_args() + args = parser.parse_args() + if args.max_trades_per_candle <= 0: + parser.error("--max-trades-per-candle must be positive") + return args
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/backtesting/hyperliquid_btc_ema_twap_backtest.py (2)
1-1
: Remove shebang or make file executable (Ruff EXE001).Either drop the shebang or chmod +x this file. Since it’s a module imported by others and run via
python ...
, dropping is cleaner.Apply this diff:
-#!/usr/bin/env python3
50-56
: Don’t blanket-catch ImportError for fallback import.Catching ImportError can mask real import errors inside the module. Narrow to ModuleNotFoundError so only missing-module cases fall back.
-try: - from .hyperliquid_data_fetcher import DEFAULT_INTERVAL - from .hyperliquid_data_fetcher import fetch_trades_to_csv -except ImportError: - from hyperliquid_data_fetcher import DEFAULT_INTERVAL - from hyperliquid_data_fetcher import fetch_trades_to_csv +try: + from .hyperliquid_data_fetcher import DEFAULT_INTERVAL + from .hyperliquid_data_fetcher import fetch_trades_to_csv +except ModuleNotFoundError: + from hyperliquid_data_fetcher import DEFAULT_INTERVAL + from hyperliquid_data_fetcher import fetch_trades_to_csv
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/backtesting/hyperliquid_btc_ema_twap_backtest.py
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/backtesting/hyperliquid_btc_ema_twap_backtest.py (1)
src/backtesting/hyperliquid_data_fetcher.py (2)
fetch_trades_to_csv
(131-152)main
(188-198)
🪛 Ruff (0.13.1)
src/backtesting/hyperliquid_btc_ema_twap_backtest.py
1-1: Shebang is present but file is not executable
(EXE001)
🔇 Additional comments (3)
src/backtesting/hyperliquid_btc_ema_twap_backtest.py (3)
141-148
: LGTM: fixed the earlier brokenfetch_trades_to_csv
call.The call now passes all required keywords and matches the fetcher signature.
118-123
: Validate--max-trades-per-candle
> 0 (crash risk on negative values).Negative/zero can break data generation. Enforce positivity at parse time.
- return parser.parse_args() + args = parser.parse_args() + if args.max_trades_per_candle <= 0: + parser.error("--max-trades-per-candle must be positive") + return args
126-133
: Unnecessary fallback guard for format="mixed" The project pins pandas >= 2.2.0 and support forformat="mixed"
was introduced in pandas 2.0.0, so the try/except fallback is redundant.Likely an incorrect or invalid review comment.
Summary by CodeRabbit
New Features
Documentation
Tests
Chores