Skip to content

Commit c5e4f5c

Browse files
committed
refactor: simplify validate_symbol to only check target != trading_symbol (#247)
validate_symbol now only prevents nonsensical orders where target_symbol equals trading_symbol (e.g. EUR/EUR). No data source lookups or symbol format construction needed.
1 parent 837d86b commit c5e4f5c

2 files changed

Lines changed: 57 additions & 90 deletions

File tree

investing_algorithm_framework/app/context.py

Lines changed: 16 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -53,34 +53,28 @@ def __init__(
5353

5454
def _validate_target_symbol(self, target_symbol, market=None):
5555
"""
56-
Validate that the target_symbol is a known symbol by checking
57-
against registered data source symbols.
56+
Validate that the target_symbol combined with the portfolio's
57+
trading_symbol does not result in trading_symbol/trading_symbol
58+
(e.g., EUR/EUR), which is a nonsensical order.
5859
5960
Args:
60-
target_symbol: The symbol to validate (e.g., "BTC" or "BTC/EUR")
61+
target_symbol: The symbol of the asset to trade
6162
market: The market to check against
6263
6364
Raises:
64-
OperationalException: If the symbol is not recognized
65+
OperationalException: If target_symbol equals
66+
the trading_symbol
6567
"""
66-
known_symbols = set()
67-
68-
# Collect symbols from registered data sources
69-
if self.data_provider_service.data_provider_index is not None:
70-
for data_source, _ in \
71-
self.data_provider_service \
72-
.data_provider_index.get_all():
73-
if data_source.symbol is not None:
74-
known_symbols.add(data_source.symbol.upper())
75-
76-
target = target_symbol.upper()
68+
portfolio = self.portfolio_service.find({"market": market})
69+
trading_symbol = portfolio.trading_symbol.upper()
7770

78-
if target not in known_symbols:
79-
sorted_symbols = sorted(known_symbols)
71+
if target_symbol.upper() == trading_symbol:
8072
raise OperationalException(
81-
f"Symbol '{target_symbol}' is not a known asset. "
82-
f"Known symbols: {sorted_symbols}. "
83-
f"Check for typos or add a DataSource for this symbol. "
73+
f"target_symbol '{target_symbol}' is the same as "
74+
f"the trading_symbol '{portfolio.trading_symbol}'. "
75+
f"This would result in a "
76+
f"'{portfolio.trading_symbol}/{portfolio.trading_symbol}' "
77+
f"order which is not valid. "
8478
f"To skip this check, set validate_symbol=False "
8579
f"or omit the parameter."
8680
)
@@ -132,7 +126,7 @@ def create_order(
132126
sync: If set to True, the created order will be synced
133127
with the portfolio of the algorithm.
134128
validate_symbol: Default False. If set to True,
135-
the target_symbol will be validated against known symbols.
129+
validates that target_symbol is not the trading_symbol.
136130
137131
Returns:
138132
The order created
@@ -235,8 +229,7 @@ def create_limit_order(
235229
the created order will be synced with the
236230
portfolio of the algorithm
237231
validate_symbol (optional): Default False. If set to True,
238-
the target_symbol will be validated against known symbols
239-
from registered data sources.
232+
validates that target_symbol is not the trading_symbol.
240233
241234
Returns:
242235
Order: Instance of the order created
Lines changed: 41 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,16 @@
11
from investing_algorithm_framework import PortfolioConfiguration, \
2-
MarketCredential, OrderSide, DataSource
2+
MarketCredential, OrderSide
33
from investing_algorithm_framework.domain import OperationalException
44
from tests.resources import TestBase
55
from tests.resources.strategies_for_testing import StrategyOne
66

77

88
class TestValidateSymbol(TestBase):
9-
"""Tests for opt-in symbol validation on order creation (issue #247)."""
9+
"""Tests for opt-in symbol validation on order creation (issue #247).
10+
11+
Validates that target_symbol is not the same as trading_symbol,
12+
which would result in a nonsensical order (e.g. EUR/EUR).
13+
"""
1014
portfolio_configurations = [
1115
PortfolioConfiguration(
1216
market="BITVAVO",
@@ -24,124 +28,94 @@ class TestValidateSymbol(TestBase):
2428
"EUR": 1000
2529
}
2630

27-
def _register_data_source(self, symbol):
28-
"""Register a data source symbol in the data provider index."""
29-
data_source = DataSource(
30-
identifier=f"{symbol}_ohlcv",
31-
symbol=symbol,
32-
data_type="OHLCV",
33-
time_frame="1d",
34-
market="BITVAVO",
35-
)
36-
data_provider_service = self.app.container.data_provider_service()
37-
data_provider_service.data_provider_index\
38-
.data_providers_lookup[data_source] = None
39-
40-
def test_default_allows_unknown_symbol(self):
41-
"""Default behavior: no symbol validation, any symbol is accepted."""
31+
def test_default_allows_trading_symbol_as_target(self):
32+
"""Default behavior: no validation, even trading_symbol is accepted."""
4233
self.app.add_strategy(StrategyOne)
4334
self.app.context.create_limit_order(
44-
target_symbol="UNKNOWN_TOKEN",
35+
target_symbol="EUR",
4536
amount=1,
4637
price=10,
4738
order_side=OrderSide.BUY,
4839
)
4940
order_repository = self.app.container.order_repository()
50-
order = order_repository.find({"target_symbol": "UNKNOWN_TOKEN"})
41+
order = order_repository.find({"target_symbol": "EUR"})
5142
self.assertIsNotNone(order)
5243

53-
def test_validate_symbol_false_allows_unknown_symbol(self):
54-
"""Explicit validate_symbol=False allows any symbol."""
44+
def test_validate_symbol_false_allows_trading_symbol_as_target(self):
45+
"""Explicit validate_symbol=False allows trading_symbol as target."""
5546
self.app.add_strategy(StrategyOne)
5647
self.app.context.create_limit_order(
57-
target_symbol="UNKNOWN_TOKEN",
48+
target_symbol="EUR",
5849
amount=1,
5950
price=10,
6051
order_side=OrderSide.BUY,
6152
validate_symbol=False,
6253
)
6354
order_repository = self.app.container.order_repository()
64-
order = order_repository.find({"target_symbol": "UNKNOWN_TOKEN"})
55+
order = order_repository.find({"target_symbol": "EUR"})
6556
self.assertIsNotNone(order)
6657

67-
def test_validate_symbol_true_rejects_unknown_symbol(self):
68-
"""validate_symbol=True raises for a symbol not in data sources."""
58+
def test_validate_symbol_true_rejects_trading_symbol_as_target(self):
59+
"""validate_symbol=True raises when target_symbol equals
60+
trading_symbol (e.g. EUR/EUR)."""
6961
self.app.add_strategy(StrategyOne)
70-
self._register_data_source("BTC/EUR")
7162

7263
with self.assertRaises(OperationalException) as cm:
7364
self.app.context.create_limit_order(
74-
target_symbol="BT/EUR",
65+
target_symbol="EUR",
7566
amount=1,
7667
price=10,
7768
order_side=OrderSide.BUY,
7869
validate_symbol=True,
7970
)
8071

81-
self.assertIn("BT/EUR", str(cm.exception))
82-
self.assertIn("not a known asset", str(cm.exception))
83-
self.assertIn("BTC/EUR", str(cm.exception))
72+
error_msg = str(cm.exception)
73+
self.assertIn("EUR", error_msg)
74+
self.assertIn("trading_symbol", error_msg)
75+
self.assertIn("EUR/EUR", error_msg)
8476

85-
def test_validate_symbol_true_accepts_known_symbol(self):
86-
"""validate_symbol=True passes for a symbol in data sources."""
77+
def test_validate_symbol_true_case_insensitive(self):
78+
"""validate_symbol=True catches trading_symbol regardless of case."""
8779
self.app.add_strategy(StrategyOne)
88-
self._register_data_source("BTC/EUR")
8980

90-
self.app.context.create_limit_order(
91-
target_symbol="BTC/EUR",
92-
amount=1,
93-
price=10,
94-
order_side=OrderSide.BUY,
95-
validate_symbol=True,
96-
)
97-
order_repository = self.app.container.order_repository()
98-
order = order_repository.find({"target_symbol": "BTC/EUR"})
99-
self.assertIsNotNone(order)
81+
with self.assertRaises(OperationalException):
82+
self.app.context.create_limit_order(
83+
target_symbol="eur",
84+
amount=1,
85+
price=10,
86+
order_side=OrderSide.BUY,
87+
validate_symbol=True,
88+
)
10089

101-
def test_validate_symbol_true_multiple_data_sources(self):
102-
"""validate_symbol=True checks across all registered data sources."""
90+
def test_validate_symbol_true_accepts_valid_target(self):
91+
"""validate_symbol=True passes for a target that differs
92+
from trading_symbol."""
10393
self.app.add_strategy(StrategyOne)
104-
self._register_data_source("BTC/EUR")
105-
self._register_data_source("ETH/EUR")
10694

107-
# ETH/EUR should pass
10895
self.app.context.create_limit_order(
109-
target_symbol="ETH/EUR",
96+
target_symbol="BTC",
11097
amount=1,
11198
price=10,
11299
order_side=OrderSide.BUY,
113100
validate_symbol=True,
114101
)
115102
order_repository = self.app.container.order_repository()
116-
order = order_repository.find({"target_symbol": "ETH/EUR"})
103+
order = order_repository.find({"target_symbol": "BTC"})
117104
self.assertIsNotNone(order)
118105

119-
# SOL/EUR should fail
120-
with self.assertRaises(OperationalException):
121-
self.app.context.create_limit_order(
122-
target_symbol="SOL/EUR",
123-
amount=1,
124-
price=10,
125-
order_side=OrderSide.BUY,
126-
validate_symbol=True,
127-
)
128-
129-
def test_validate_symbol_error_message_contains_known_symbols(self):
130-
"""Error message lists known symbols for user reference."""
106+
def test_validate_symbol_error_message(self):
107+
"""Error message explains the EUR/EUR problem and how to skip."""
131108
self.app.add_strategy(StrategyOne)
132-
self._register_data_source("BTC/EUR")
133-
self._register_data_source("ETH/EUR")
134109

135110
with self.assertRaises(OperationalException) as cm:
136111
self.app.context.create_limit_order(
137-
target_symbol="TYPO",
112+
target_symbol="EUR",
138113
amount=1,
139114
price=10,
140115
order_side=OrderSide.BUY,
141116
validate_symbol=True,
142117
)
143118

144119
error_msg = str(cm.exception)
145-
self.assertIn("BTC/EUR", error_msg)
146-
self.assertIn("ETH/EUR", error_msg)
120+
self.assertIn("EUR/EUR", error_msg)
147121
self.assertIn("validate_symbol=False", error_msg)

0 commit comments

Comments
 (0)