diff --git a/.agents/skills/pr-feedback-triage/SKILL.md b/.agents/skills/pr-feedback-triage/SKILL.md index 2622814..fa54f79 100644 --- a/.agents/skills/pr-feedback-triage/SKILL.md +++ b/.agents/skills/pr-feedback-triage/SKILL.md @@ -104,8 +104,8 @@ A reliable pattern is: Example GraphQL mutation shape: ```graphql -mutation($threadId: ID!) { - resolveReviewThread(input: {threadId: $threadId}) { +mutation ($threadId: ID!) { + resolveReviewThread(input: { threadId: $threadId }) { thread { id isResolved diff --git a/docs/api/public-contract.md b/docs/api/public-contract.md index 0d16674..85f5c46 100644 --- a/docs/api/public-contract.md +++ b/docs/api/public-contract.md @@ -19,20 +19,27 @@ alias for new code. ### Session lifecycle and configuration -| Symbol | Role | -| ----------------------------------------------- | ---------------------------------------------------------------- | -| `MT5Client`, `Mt5CliClient` | Read-only data client with optional `order_check` / `order_send` | -| `build_config` | Build `pdmt5.Mt5Config` from connection fields | -| `mt5_session` | Context manager: initialize, login, yield client, shutdown | -| `create_trading_client`, `mt5_trading_session` | Trading-capable `pdmt5.Mt5TradingClient` lifecycle | -| `AccountSpec` | Generic account group: symbols plus optional credentials | -| `resolve_account_spec`, `resolve_account_specs` | Merge overrides and expand `${ENV_VAR}` placeholders | -| `substitute_env_placeholders` | Replace `${NAME}` substrings from the environment | +| Symbol | Role | +| ----------------------------------------------- | ---------------------------------------------------------------------------------------------------------- | +| `MT5Client`, `Mt5CliClient` | Read-only data client with optional `order_check` / `order_send` | +| `build_config` | Build `pdmt5.Mt5Config` from connection fields | +| `mt5_session` | Context manager: initialize, login, yield client, shutdown | +| `create_trading_client`, `mt5_trading_session` | Trading-capable `pdmt5.Mt5TradingClient` lifecycle | +| `AccountSpec` | Generic account group: symbols plus optional credentials | +| `resolve_account_spec`, `resolve_account_specs` | Merge overrides and expand `${ENV_VAR}` placeholders; opt-in `allow_whole_dollar_env` for bare `$NAME` | +| `substitute_env_placeholders` | Replace `${NAME}` substrings from the environment; opt-in `allow_whole_dollar_env` for whole-value `$NAME` | Credential resolution is generic: any environment variable name may appear inside `${...}`. mt5cli does not hard-code application-specific keys such as `mt5_login` or `mt5_exe`. +Pass `allow_whole_dollar_env=True` to `substitute_env_placeholders()`, +`resolve_account_spec()`, `resolve_account_specs()`, and `build_config()` to +additionally expand strings whose entire value is a bare `$ENV_NAME` identifier. +Partial strings such as `"plan$pass"`, `"abc$ENV"`, or `"$ENV-suffix"` are +**never** expanded — only an exact `$IDENTIFIER` whole-string match qualifies. +Default is `False` to preserve backward compatibility. + ### Read-only MT5 data access Module-level helpers open a transient connection per call. Prefer `mt5_session` @@ -56,15 +63,16 @@ MetaTrader 5 returns the still-forming bar as the last row when `start_pos=0`. Use these helpers instead of reimplementing bar trimming or timestamp normalization in downstream apps. -| Symbol | Role | -| ------------------------------------------------ | ------------------------------------------------------------ | -| `drop_forming_rate_bar` | Remove the last row from chronologically ordered rate data | -| `fetch_latest_closed_rates` | Single connected client: fetch `count + 1`, drop forming bar | -| `fetch_latest_closed_rates_for_trading_client` | Closed bars from an active `Mt5TradingClient` session | -| `collect_latest_closed_rates_for_accounts` | Multi-account closed bars with optional retry wrapper | -| `collect_latest_closed_rates_by_granularity` | Same data keyed by `(symbol, granularity_name)` | -| `collect_latest_rates_for_accounts` | Latest bars including the forming bar when `start_pos=0` | -| `collect_latest_rates_for_accounts_with_retries` | Bounded exponential backoff for transient MT5 errors | +| Symbol | Role | +| ------------------------------------------------ | ------------------------------------------------------------------------------- | +| `drop_forming_rate_bar` | Remove the last row from chronologically ordered rate data | +| `fetch_latest_closed_rates` | Single connected client: fetch `count + 1`, drop forming bar | +| `fetch_latest_closed_rates_for_trading_client` | Closed bars from an active `Mt5TradingClient` session; returns RangeIndex | +| `fetch_latest_closed_rates_indexed` | Same as above but returns a UTC `DatetimeIndex` named `"time"` (no time column) | +| `collect_latest_closed_rates_for_accounts` | Multi-account closed bars with optional retry wrapper | +| `collect_latest_closed_rates_by_granularity` | Same data keyed by `(symbol, granularity_name)` | +| `collect_latest_rates_for_accounts` | Latest bars including the forming bar when `start_pos=0` | +| `collect_latest_rates_for_accounts_with_retries` | Bounded exponential backoff for transient MT5 errors | ### SQLite history collection and rate loading @@ -97,7 +105,7 @@ strategy entries, exits, Kelly sizing, or signal logic. | `detect_position_side` | Net long / short / flat from open positions | | `calculate_spread_ratio` | Relative bid-ask spread | | `calculate_margin_and_volume`, `calculate_volume_by_margin`, `calculate_new_position_margin_ratio` | Margin budget and volume sizing | -| `normalize_order_volume`, `estimate_order_margin`, `calculate_positions_margin` | Broker volume normalization and margin totals | +| `normalize_order_volume`, `estimate_order_margin`, `calculate_positions_margin` | Broker volume normalization and margin totals | | `determine_order_limits` | SL/TP price levels from ratios | | `ensure_symbol_selected` | Select/verify Market Watch visibility | | `place_market_order`, `close_open_positions`, `update_sltp_for_open_positions` | Order execution helpers (`dry_run` supported) | diff --git a/docs/api/sdk.md b/docs/api/sdk.md index d051f87..e4d8b63 100644 --- a/docs/api/sdk.md +++ b/docs/api/sdk.md @@ -86,6 +86,28 @@ resolved = resolve_account_specs(accounts, server="Broker-Demo") # resolved[0].login == "12345", resolved[0].server == "Broker-Demo" ``` +Pass `allow_whole_dollar_env=True` to also expand strings whose **entire value** +is a bare `$ENV_NAME` identifier (no braces). This opt-in covers +`substitute_env_placeholders()`, `resolve_account_spec()`, +`resolve_account_specs()`, and `build_config()`. Note: `build_config` cannot +expand `login` because that parameter is `int | None`; use +`resolve_account_spec` for a string `login` placeholder. Partial strings such as +`"plan$pass"`, `"abc$ENV"`, or `"$ENV-suffix"` are never expanded — only an +exact `$IDENTIFIER` whole-string match qualifies. The default is `False` to +preserve backward compatibility. + +```python +import os + +from mt5cli import AccountSpec, resolve_account_specs + +os.environ["MT5_PASSWORD"] = "secret" +accounts = [AccountSpec(symbols=["EURUSD"], password="$MT5_PASSWORD")] + +resolved = resolve_account_specs(accounts, allow_whole_dollar_env=True) +# resolved[0].password == "secret" +``` + ### Throttled incremental history updates `ThrottledHistoryUpdater` wraps `update_history()` with a minimum interval diff --git a/docs/api/trading.md b/docs/api/trading.md index 747ec77..a2ff4d2 100644 --- a/docs/api/trading.md +++ b/docs/api/trading.md @@ -48,6 +48,7 @@ from mt5cli import ( determine_order_limits, estimate_order_margin, fetch_latest_closed_rates_for_trading_client, + fetch_latest_closed_rates_indexed, get_account_snapshot, get_positions_frame, get_symbol_snapshot, @@ -78,6 +79,14 @@ closed_bars = fetch_latest_closed_rates_for_trading_client( granularity="M1", count=100, ) +# Or fetch with a UTC DatetimeIndex instead of a "time" column: +indexed_bars = fetch_latest_closed_rates_indexed( + client, + symbol="EURUSD", + granularity="M1", + count=100, +) +# indexed_bars.index is a UTC-aware DatetimeIndex named "time" sizing = calculate_margin_and_volume( client, "EURUSD", @@ -174,16 +183,16 @@ through the stable package root without embedding entry/exit policy. ## Migration from application-local helpers -| Application-local concern | mt5cli replacement | -| -------------------------------------------------------- | ----------------------------------------------- | -| Manual terminal spawn/kill around trading code | `mt5_trading_session()` | -| Local position-side detection | `detect_position_side()` | -| Local margin/volume sizing | `calculate_margin_and_volume()` | -| Local broker volume step normalization | `normalize_order_volume()` | -| Local order or position margin estimation | `estimate_order_margin()`, `calculate_positions_margin()` | -| Local closed-bar fetch from a trading session | `fetch_latest_closed_rates_for_trading_client()` | -| Local SL/TP price derivation | `determine_order_limits()` | -| Throttled SQLite history loop with ad-hoc error handling | `ThrottledHistoryUpdater(suppress_errors=True)` | +| Application-local concern | mt5cli replacement | +| -------------------------------------------------------- | --------------------------------------------------------------------------------------- | +| Manual terminal spawn/kill around trading code | `mt5_trading_session()` | +| Local position-side detection | `detect_position_side()` | +| Local margin/volume sizing | `calculate_margin_and_volume()` | +| Local broker volume step normalization | `normalize_order_volume()` | +| Local order or position margin estimation | `estimate_order_margin()`, `calculate_positions_margin()` | +| Local closed-bar fetch from a trading session | `fetch_latest_closed_rates_for_trading_client()`, `fetch_latest_closed_rates_indexed()` | +| Local SL/TP price derivation | `determine_order_limits()` | +| Throttled SQLite history loop with ad-hoc error handling | `ThrottledHistoryUpdater(suppress_errors=True)` | Keep read-only data collection on `mt5_session()` / `Mt5CliClient`; use `mt5_trading_session()` only where order placement or trading calculations are diff --git a/mt5cli/__init__.py b/mt5cli/__init__.py index bd66dd7..4d32dbf 100644 --- a/mt5cli/__init__.py +++ b/mt5cli/__init__.py @@ -128,6 +128,7 @@ from .trading import ( ensure_symbol_selected, estimate_order_margin, fetch_latest_closed_rates_for_trading_client, + fetch_latest_closed_rates_indexed, get_account_snapshot, get_positions_frame, get_symbol_snapshot, @@ -214,6 +215,7 @@ __all__ = [ "export_dataframe_to_sqlite", "fetch_latest_closed_rates", "fetch_latest_closed_rates_for_trading_client", + "fetch_latest_closed_rates_indexed", "get_account_snapshot", "get_positions_frame", "get_symbol_snapshot", diff --git a/mt5cli/contract.py b/mt5cli/contract.py index 6fb410f..b058ab2 100644 --- a/mt5cli/contract.py +++ b/mt5cli/contract.py @@ -56,6 +56,7 @@ STABLE_SDK_EXPORTS: frozenset[str] = frozenset({ "export_dataframe_to_sqlite", "fetch_latest_closed_rates", "fetch_latest_closed_rates_for_trading_client", + "fetch_latest_closed_rates_indexed", "get_account_snapshot", "get_positions_frame", "get_symbol_snapshot", diff --git a/mt5cli/sdk.py b/mt5cli/sdk.py index 94e92db..9e93377 100644 --- a/mt5cli/sdk.py +++ b/mt5cli/sdk.py @@ -309,12 +309,33 @@ def build_config( password: str | None = None, server: str | None = None, timeout: int | None = None, + allow_whole_dollar_env: bool = False, ) -> Mt5Config: """Build an ``Mt5Config`` from optional connection parameters. + Args: + path: Optional terminal executable path. + login: Optional trading account login. + password: Optional trading account password. + server: Optional trading server name. + timeout: Optional connection timeout in milliseconds. + allow_whole_dollar_env: When ``True``, string parameters that are + exactly ``$ENV_NAME`` are expanded from the environment. Applies + to ``path``, ``password``, and ``server``. Default ``False`` + preserves existing behavior. + Returns: Configured ``Mt5Config`` instance. """ + if allow_whole_dollar_env: + if path is not None: + path = substitute_env_placeholders(path, allow_whole_dollar_env=True) + if password is not None: + password = substitute_env_placeholders( + password, allow_whole_dollar_env=True + ) + if server is not None: + server = substitute_env_placeholders(server, allow_whole_dollar_env=True) return Mt5Config( path=path, login=login, @@ -1376,13 +1397,22 @@ class AccountSpec: _ENV_PLACEHOLDER_PATTERN = re.compile(r"\$\{(?P[A-Za-z_][A-Za-z0-9_]*)\}") +_WHOLE_DOLLAR_PATTERN = re.compile(r"^\$(?P[A-Za-z_][A-Za-z0-9_]*)$") -def substitute_env_placeholders(value: str) -> str: +def substitute_env_placeholders( + value: str, + *, + allow_whole_dollar_env: bool = False, +) -> str: """Replace ``${ENV_VAR}`` placeholders in a string with environment values. Args: value: String that may contain one or more ``${ENV_VAR}`` placeholders. + allow_whole_dollar_env: When ``True``, a string that is exactly + ``$ENV_NAME`` (the whole value and nothing else) is also expanded + from the environment. Partial occurrences such as ``"plan$pass"`` + or ``"$ENV-suffix"`` are left unchanged. Returns: The string with every placeholder replaced by its environment value. @@ -1390,6 +1420,14 @@ def substitute_env_placeholders(value: str) -> str: Raises: ValueError: If a referenced environment variable is not set. """ + if allow_whole_dollar_env: + m = _WHOLE_DOLLAR_PATTERN.match(value) + if m: + name = m.group("name") + if name not in os.environ: + msg = f"Environment variable {name!r} is not set." + raise ValueError(msg) + return os.environ[name] parts: list[str] = [] last_end = 0 for match in _ENV_PLACEHOLDER_PATTERN.finditer(value): @@ -1404,7 +1442,12 @@ def substitute_env_placeholders(value: str) -> str: return "".join(parts) -def _resolve_field(override: str | None, account_value: str | None) -> str | None: +def _resolve_field( + override: str | None, + account_value: str | None, + *, + allow_whole_dollar_env: bool = False, +) -> str | None: """Resolve a string field from an override or account value with env subst. Returns: @@ -1414,12 +1457,16 @@ def _resolve_field(override: str | None, account_value: str | None) -> str | Non value = override if override is not None else account_value if value is None: return None - return substitute_env_placeholders(value) + return substitute_env_placeholders( + value, allow_whole_dollar_env=allow_whole_dollar_env + ) def _resolve_login( override: int | str | None, account_login: int | str | None, + *, + allow_whole_dollar_env: bool = False, ) -> int | str | None: """Resolve a login from an override or account value with env substitution. @@ -1431,10 +1478,14 @@ def _resolve_login( if override is not None: if isinstance(override, int): return override - return substitute_env_placeholders(override) + return substitute_env_placeholders( + override, allow_whole_dollar_env=allow_whole_dollar_env + ) if account_login is None or isinstance(account_login, int): return account_login - return substitute_env_placeholders(account_login) + return substitute_env_placeholders( + account_login, allow_whole_dollar_env=allow_whole_dollar_env + ) def resolve_account_spec( @@ -1445,6 +1496,7 @@ def resolve_account_spec( server: str | None = None, path: str | None = None, timeout: int | None = None, + allow_whole_dollar_env: bool = False, ) -> AccountSpec: """Resolve an account's credentials from overrides and ``${ENV_VAR}`` values. @@ -1460,6 +1512,9 @@ def resolve_account_spec( server: Optional explicit server override. path: Optional explicit terminal path override. timeout: Optional explicit connection timeout override. + allow_whole_dollar_env: When ``True``, string fields that are exactly + ``$ENV_NAME`` are also expanded from the environment. Default + ``False`` preserves existing behavior. Returns: A new :class:`AccountSpec` with resolved credentials and the original @@ -1469,10 +1524,18 @@ def resolve_account_spec( """ return AccountSpec( symbols=account.symbols, - login=_resolve_login(login, account.login), - password=_resolve_field(password, account.password), - server=_resolve_field(server, account.server), - path=_resolve_field(path, account.path), + login=_resolve_login( + login, account.login, allow_whole_dollar_env=allow_whole_dollar_env + ), + password=_resolve_field( + password, account.password, allow_whole_dollar_env=allow_whole_dollar_env + ), + server=_resolve_field( + server, account.server, allow_whole_dollar_env=allow_whole_dollar_env + ), + path=_resolve_field( + path, account.path, allow_whole_dollar_env=allow_whole_dollar_env + ), timeout=timeout if timeout is not None else account.timeout, ) @@ -1485,6 +1548,7 @@ def resolve_account_specs( server: str | None = None, path: str | None = None, timeout: int | None = None, + allow_whole_dollar_env: bool = False, ) -> list[AccountSpec]: """Resolve credentials for multiple accounts. @@ -1498,6 +1562,9 @@ def resolve_account_specs( server: Optional explicit server override applied to each account. path: Optional explicit terminal path override applied to each account. timeout: Optional explicit timeout override applied to each account. + allow_whole_dollar_env: When ``True``, string fields that are exactly + ``$ENV_NAME`` are also expanded from the environment. Default + ``False`` preserves existing behavior. Returns: Resolved account specifications in the original order. Raises @@ -1512,6 +1579,7 @@ def resolve_account_specs( server=server, path=path, timeout=timeout, + allow_whole_dollar_env=allow_whole_dollar_env, ) for account in accounts ] diff --git a/mt5cli/trading.py b/mt5cli/trading.py index a286b42..c5e0d3c 100644 --- a/mt5cli/trading.py +++ b/mt5cli/trading.py @@ -137,6 +137,7 @@ __all__ = [ "ensure_symbol_selected", "estimate_order_margin", "fetch_latest_closed_rates_for_trading_client", + "fetch_latest_closed_rates_indexed", "get_account_snapshot", "get_positions_frame", "get_symbol_snapshot", @@ -1202,6 +1203,94 @@ def fetch_latest_closed_rates_for_trading_client( return closed.tail(count).reset_index(drop=True) +def _rate_time_to_utc(series: pd.Series, symbol: str) -> pd.DatetimeIndex: + """Convert a rate time series to a UTC-aware DatetimeIndex. + + Handles MT5 epoch seconds (including object-dtype Python numbers), timezone- + naive datetime-like values, and timezone-aware datetime-like values. + + Returns: + UTC-aware DatetimeIndex. + + Raises: + ValueError: If the time data is invalid, unparseable, or contains NaT. + """ + try: + arr = series.to_numpy() + non_null = series.dropna() + object_numbers = ( + pd.api.types.is_object_dtype(series) + and non_null.map( + lambda value: ( + isinstance(value, int | float) and not isinstance(value, bool) + ), + ).all() + ) + numeric_dtype = pd.api.types.is_numeric_dtype( + series + ) and not pd.api.types.is_bool_dtype( + series, + ) + if numeric_dtype or object_numbers: + idx = pd.to_datetime(arr, unit="s", utc=True) + else: + idx = pd.to_datetime(arr, utc=True) + except Exception as exc: + msg = f"Rate data for {symbol!r} has invalid or unparseable time data." + raise ValueError(msg) from exc + if any(idx.isna()): + msg = f"Rate data for {symbol!r} contains missing (NaT) timestamp values." + raise ValueError(msg) + return idx + + +def fetch_latest_closed_rates_indexed( + client: Mt5TradingClient, + *, + symbol: str, + granularity: str, + count: int, +) -> pd.DataFrame: + """Fetch the latest closed bars with a UTC DatetimeIndex from a trading client. + + Internally reuses :func:`fetch_latest_closed_rates_for_trading_client` for + closed-bar detection and validation, then converts the ``time`` column to a + UTC-aware :class:`~pandas.DatetimeIndex` named ``"time"`` and drops the + original column. Intended for downstream time-series consumers that require + a datetime index rather than a ``time`` column. + + Args: + client: Connected trading client with rate-fetch capability. + symbol: Symbol name. + granularity: Timeframe string (for example ``"M1"``, ``"H1"``). + count: Maximum number of closed bars to return. + + Returns: + Up to ``count`` closed bars ordered oldest to newest, with a + UTC-aware ``DatetimeIndex`` named ``"time"``. The original ``time`` + column is dropped. + + Raises: + ValueError: If ``count`` is not positive, rate data is empty or + malformed, the ``time`` column is missing, or timestamp data + is invalid or unparseable. + """ + frame = fetch_latest_closed_rates_for_trading_client( + client, + symbol=symbol, + granularity=granularity, + count=count, + ) + if "time" not in frame.columns: + msg = f"Rate data is missing a time column for {symbol!r}." + raise ValueError(msg) + idx = _rate_time_to_utc(frame["time"], symbol) + idx.name = "time" + result = frame.drop(columns=["time"]) + result.index = idx + return result + + @contextmanager def mt5_trading_session( config: Mt5Config | None = None, diff --git a/pyproject.toml b/pyproject.toml index 096c4ba..b8ba49a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "mt5cli" -version = "0.8.3" +version = "0.9.0" description = "Generic MT5 data and execution infrastructure for Python applications" authors = [{name = "dceoy", email = "dceoy@users.noreply.github.com"}] maintainers = [{name = "dceoy", email = "dceoy@users.noreply.github.com"}] diff --git a/tests/test_contracts.py b/tests/test_contracts.py index 554e8da..4e932c6 100644 --- a/tests/test_contracts.py +++ b/tests/test_contracts.py @@ -44,6 +44,7 @@ from mt5cli import ( export_dataframe_to_sqlite, fetch_latest_closed_rates, fetch_latest_closed_rates_for_trading_client, + fetch_latest_closed_rates_indexed, granularity_name, is_recoverable_mt5_error, load_rate_data, @@ -734,3 +735,32 @@ class TestStableSdkContract: raise RuntimeError(message) mock_client.shutdown.assert_called_once() + + def test_fetch_latest_closed_rates_indexed_from_package_root( + self, + mocker: MockerFixture, + ) -> None: + """Indexed closed-bar helper returns a UTC DatetimeIndex named 'time'.""" + client = MagicMock() + mocker.patch( + "mt5cli.trading.fetch_latest_closed_rates_for_trading_client", + return_value=pd.DataFrame( + { + "time": [1704067200, 1704153600, 1704240000], + "close": [1.0, 1.1, 1.2], + }, + ), + ) + + result = fetch_latest_closed_rates_indexed( + client, + symbol="EURUSD", + granularity="M1", + count=2, + ) + + assert isinstance(result.index, pd.DatetimeIndex) + assert result.index.name == "time" + assert result.index.tz is not None + assert "time" not in result.columns + assert "close" in result.columns diff --git a/tests/test_sdk.py b/tests/test_sdk.py index 5872934..60e831f 100644 --- a/tests/test_sdk.py +++ b/tests/test_sdk.py @@ -1777,6 +1777,80 @@ class TestSubstituteEnvPlaceholders: with pytest.raises(ValueError, match="'MT5_MISSING' is not set"): substitute_env_placeholders("${MT5_MISSING}") + def test_whole_dollar_not_substituted_by_default( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """Test $ENV_NAME is not expanded without allow_whole_dollar_env=True.""" + monkeypatch.setenv("MT5_PASSWORD", "secret") + + assert substitute_env_placeholders("$MT5_PASSWORD") == "$MT5_PASSWORD" + + def test_whole_dollar_substituted_with_opt_in( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """Test $ENV_NAME is expanded when allow_whole_dollar_env=True.""" + monkeypatch.setenv("MT5_PASSWORD", "secret") + + result = substitute_env_placeholders( + "$MT5_PASSWORD", allow_whole_dollar_env=True + ) + + assert result == "secret" + + def test_whole_dollar_missing_variable_raises_value_error( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """Test missing $ENV_NAME raises ValueError when opt-in is enabled.""" + monkeypatch.delenv("MT5_MISSING", raising=False) + + with pytest.raises(ValueError, match="'MT5_MISSING' is not set"): + substitute_env_placeholders("$MT5_MISSING", allow_whole_dollar_env=True) + + def test_partial_dollar_not_expanded_with_opt_in( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """Test $ENV embedded in a larger string is not expanded.""" + monkeypatch.setenv("pass", "secret") + monkeypatch.setenv("ENV", "val") + + assert ( + substitute_env_placeholders("plan$pass", allow_whole_dollar_env=True) + == "plan$pass" + ) + assert ( + substitute_env_placeholders("abc$ENV", allow_whole_dollar_env=True) + == "abc$ENV" + ) + + def test_dollar_with_suffix_not_expanded_with_opt_in( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """Test $ENV-suffix is not expanded (not a whole-value placeholder).""" + monkeypatch.setenv("ENV", "val") + + assert ( + substitute_env_placeholders("$ENV-suffix", allow_whole_dollar_env=True) + == "$ENV-suffix" + ) + + def test_brace_format_works_with_opt_in( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """Test ${ENV_VAR} substitution still works when allow_whole_dollar_env=True.""" + monkeypatch.setenv("MT5_LOGIN", "12345") + + result = substitute_env_placeholders( + "${MT5_LOGIN}", allow_whole_dollar_env=True + ) + + assert result == "12345" + class TestResolveAccountSpec: """Tests for resolve_account_spec and resolve_account_specs.""" @@ -1863,6 +1937,117 @@ class TestResolveAccountSpec: assert [a.server for a in resolved] == ["Shared", "Fixed"] assert all(a.timeout == 1000 for a in resolved) + def test_resolve_account_spec_with_whole_dollar_env( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """Account spec expands $ENV_NAME when allow_whole_dollar_env=True.""" + monkeypatch.setenv("MT5_PASSWORD", "secret") + account = AccountSpec(symbols=["EURUSD"], password="$MT5_PASSWORD") + + resolved = resolve_account_spec(account, allow_whole_dollar_env=True) + + assert resolved.password == "secret" # noqa: S105 + + def test_resolve_account_spec_whole_dollar_not_expanded_by_default( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """Test resolve_account_spec leaves $ENV_NAME literal by default.""" + monkeypatch.setenv("MT5_PASSWORD", "secret") + account = AccountSpec(symbols=["EURUSD"], password="$MT5_PASSWORD") + + resolved = resolve_account_spec(account) + + assert resolved.password == "$MT5_PASSWORD" # noqa: S105 + + def test_resolve_account_specs_with_whole_dollar_env( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """Test resolve_account_specs threads allow_whole_dollar_env to each account.""" + monkeypatch.setenv("MT5_SERVER", "Broker-Demo") + accounts = [ + AccountSpec(symbols=["EURUSD"], server="$MT5_SERVER"), + AccountSpec(symbols=["GBPUSD"], server="Fixed"), + ] + + resolved = resolve_account_specs(accounts, allow_whole_dollar_env=True) + + assert resolved[0].server == "Broker-Demo" + assert resolved[1].server == "Fixed" + + def test_resolve_account_spec_whole_dollar_login( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """Test $ENV_NAME login string is expanded when allow_whole_dollar_env=True.""" + monkeypatch.setenv("MT5_LOGIN", "12345") + account = AccountSpec(symbols=["EURUSD"], login="$MT5_LOGIN") + + resolved = resolve_account_spec(account, allow_whole_dollar_env=True) + + assert resolved.login == "12345" + + +class TestBuildConfigWholeDollarEnv: + """Tests for build_config with allow_whole_dollar_env.""" + + def test_build_config_substitutes_server_with_opt_in( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """build_config expands $ENV_NAME server when allow_whole_dollar_env=True.""" + monkeypatch.setenv("MT5_SERVER", "Broker-Demo") + + config = build_config(server="$MT5_SERVER", allow_whole_dollar_env=True) + + assert config.server == "Broker-Demo" + + def test_build_config_substitutes_password_with_opt_in( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """build_config expands $ENV_NAME password when allow_whole_dollar_env=True.""" + monkeypatch.setenv("MT5_PASSWORD", "secret") + + config = build_config(password="$MT5_PASSWORD", allow_whole_dollar_env=True) + + assert config.password == "secret" # noqa: S105 + + def test_build_config_substitutes_path_with_opt_in( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """Test build_config expands $ENV_NAME path when allow_whole_dollar_env=True.""" + monkeypatch.setenv("MT5_PATH", "/opt/mt5/terminal64.exe") + + config = build_config(path="$MT5_PATH", allow_whole_dollar_env=True) + + assert config.path == "/opt/mt5/terminal64.exe" + + def test_build_config_leaves_dollar_literal_by_default( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """Test build_config does not substitute $ENV without opt-in.""" + monkeypatch.setenv("MT5_SERVER", "Broker-Demo") + + config = build_config(server="$MT5_SERVER") + + assert config.server == "$MT5_SERVER" + + def test_build_config_none_params_not_substituted( + self, + monkeypatch: pytest.MonkeyPatch, # noqa: ARG002 + ) -> None: + """Test build_config with None params does not raise even with opt-in.""" + config = build_config(allow_whole_dollar_env=True) + + assert config.server is None + assert config.password is None + assert config.path is None + class TestThrottledHistoryUpdater: """Tests for the throttled incremental history updater.""" diff --git a/tests/test_trading.py b/tests/test_trading.py index aa77b19..ebb5016 100644 --- a/tests/test_trading.py +++ b/tests/test_trading.py @@ -29,6 +29,7 @@ from mt5cli.trading import ( ensure_symbol_selected, estimate_order_margin, fetch_latest_closed_rates_for_trading_client, + fetch_latest_closed_rates_indexed, get_account_snapshot, get_positions_frame, get_symbol_snapshot, @@ -2627,3 +2628,265 @@ class TestFetchLatestClosedRatesForTradingClient: ) client.fetch_latest_rates_as_df.assert_not_called() + + def test_returns_range_index_and_time_column_for_backward_compat(self) -> None: + """Test original helper returns RangeIndex with a time column.""" + client = MagicMock() + client.fetch_latest_rates_as_df.return_value = pd.DataFrame( + {"time": [1700000000, 1700003600, 1700007200], "close": [1.1, 1.2, 1.3]}, + ) + + result = fetch_latest_closed_rates_for_trading_client( + client, + symbol="EURUSD", + granularity="M1", + count=2, + ) + + assert isinstance(result.index, pd.RangeIndex) + assert "time" in result.columns + assert len(result) == 2 + + +class TestFetchLatestClosedRatesIndexed: + """Tests for fetch_latest_closed_rates_indexed.""" + + def test_converts_epoch_seconds_to_utc_datetime_index( + self, mocker: MockerFixture + ) -> None: + """Test integer epoch second timestamps become a UTC DatetimeIndex.""" + frame = pd.DataFrame( + {"time": [1700000000, 1700003600], "close": [1.1, 1.2]}, + ) + mocker.patch( + "mt5cli.trading.fetch_latest_closed_rates_for_trading_client", + return_value=frame, + ) + + result = fetch_latest_closed_rates_indexed( + MagicMock(), + symbol="EURUSD", + granularity="M1", + count=2, + ) + + assert isinstance(result.index, pd.DatetimeIndex) + assert result.index.name == "time" + assert result.index.tz is not None + assert str(result.index.tz) == "UTC" + assert "time" not in result.columns + assert list(result["close"]) == [1.1, 1.2] + + def test_converts_float_epoch_seconds_to_utc_datetime_index( + self, mocker: MockerFixture + ) -> None: + """Test float64 epoch second timestamps (after concat/NA upcast) become UTC.""" + frame = pd.DataFrame( + {"time": [1700000000.0, 1700003600.0], "close": [1.1, 1.2]}, + ) + mocker.patch( + "mt5cli.trading.fetch_latest_closed_rates_for_trading_client", + return_value=frame, + ) + + result = fetch_latest_closed_rates_indexed( + MagicMock(), + symbol="EURUSD", + granularity="M1", + count=2, + ) + + assert isinstance(result.index, pd.DatetimeIndex) + assert result.index.tz is not None + assert str(result.index.tz) == "UTC" + assert result.index[0].year == 2023 + + @pytest.mark.parametrize( + "timestamps", + [ + [1700000000, 1700003600], + [1700000000.0, 1700003600.0], + ], + ids=["integers", "floats"], + ) + def test_converts_object_numeric_epoch_seconds_to_utc_datetime_index( + self, mocker: MockerFixture, timestamps: list[int] | list[float] + ) -> None: + """Test object-dtype Python numbers are interpreted as epoch seconds.""" + frame = pd.DataFrame( + { + "time": pd.Series(timestamps, dtype=object), + "close": [1.1, 1.2], + }, + ) + mocker.patch( + "mt5cli.trading.fetch_latest_closed_rates_for_trading_client", + return_value=frame, + ) + + result = fetch_latest_closed_rates_indexed( + MagicMock(), symbol="EURUSD", granularity="M1", count=2 + ) + + assert list(result.index) == list( + pd.to_datetime(timestamps, unit="s", utc=True) + ) + + def test_parses_mixed_datetime_like_strings(self, mocker: MockerFixture) -> None: + """Test object-dtype datetime strings retain datetime-like parsing.""" + timestamps = ["2024-01-01T00:00:00Z", "2024-01-01T01:00:00+01:00"] + frame = pd.DataFrame({"time": timestamps, "close": [1.1, 1.2]}) + mocker.patch( + "mt5cli.trading.fetch_latest_closed_rates_for_trading_client", + return_value=frame, + ) + + result = fetch_latest_closed_rates_indexed( + MagicMock(), symbol="EURUSD", granularity="M1", count=2 + ) + + assert list(result.index) == list(pd.to_datetime(timestamps, utc=True)) + + def test_does_not_treat_bool_as_epoch_seconds(self, mocker: MockerFixture) -> None: + """Test bool timestamps do not enter the numeric epoch-seconds path.""" + frame = pd.DataFrame( + {"time": pd.Series([True, False], dtype=object), "close": [1.1, 1.2]}, + ) + mocker.patch( + "mt5cli.trading.fetch_latest_closed_rates_for_trading_client", + return_value=frame, + ) + + with pytest.raises(ValueError, match="invalid or unparseable time data"): + fetch_latest_closed_rates_indexed( + MagicMock(), symbol="EURUSD", granularity="M1", count=2 + ) + + def test_converts_naive_datetime_to_utc_datetime_index( + self, mocker: MockerFixture + ) -> None: + """Test timezone-naive datetime values are localized to UTC.""" + from datetime import datetime # noqa: PLC0415 + + frame = pd.DataFrame( + { + "time": [datetime(2024, 1, 1, 0, 0), datetime(2024, 1, 1, 1, 0)], # noqa: DTZ001 + "close": [1.1, 1.2], + }, + ) + mocker.patch( + "mt5cli.trading.fetch_latest_closed_rates_for_trading_client", + return_value=frame, + ) + + result = fetch_latest_closed_rates_indexed( + MagicMock(), + symbol="EURUSD", + granularity="M1", + count=2, + ) + + assert isinstance(result.index, pd.DatetimeIndex) + assert result.index.tz is not None + assert str(result.index.tz) == "UTC" + assert result.index[0].year == 2024 + + def test_converts_aware_datetime_to_utc(self, mocker: MockerFixture) -> None: + """Test timezone-aware datetime values are converted to UTC.""" + from datetime import datetime, timedelta, timezone # noqa: PLC0415 + + tz_plus5 = timezone(timedelta(hours=5)) + frame = pd.DataFrame( + { + "time": [datetime(2024, 1, 1, 5, 0, tzinfo=tz_plus5)], + "close": [1.1], + }, + ) + mocker.patch( + "mt5cli.trading.fetch_latest_closed_rates_for_trading_client", + return_value=frame, + ) + + result = fetch_latest_closed_rates_indexed( + MagicMock(), + symbol="EURUSD", + granularity="M1", + count=1, + ) + + assert isinstance(result.index, pd.DatetimeIndex) + assert str(result.index.tz) == "UTC" + assert result.index[0].hour == 0 + + def test_raises_on_missing_time_column(self, mocker: MockerFixture) -> None: + """Test missing time column after the underlying fetch raises ValueError.""" + mocker.patch( + "mt5cli.trading.fetch_latest_closed_rates_for_trading_client", + return_value=pd.DataFrame({"close": [1.1]}), + ) + + with pytest.raises(ValueError, match="missing a time column"): + fetch_latest_closed_rates_indexed( + MagicMock(), + symbol="EURUSD", + granularity="M1", + count=1, + ) + + def test_raises_on_unparseable_time_column(self, mocker: MockerFixture) -> None: + """Test unparseable time data raises a clear ValueError.""" + mocker.patch( + "mt5cli.trading.fetch_latest_closed_rates_for_trading_client", + return_value=pd.DataFrame({"time": ["not-a-date"], "close": [1.1]}), + ) + + with pytest.raises(ValueError, match="invalid or unparseable time data"): + fetch_latest_closed_rates_indexed( + MagicMock(), + symbol="EURUSD", + granularity="M1", + count=1, + ) + + def test_raises_on_nat_time_column(self, mocker: MockerFixture) -> None: + """Test NaT in the time column raises ValueError instead of silently passing.""" + mocker.patch( + "mt5cli.trading.fetch_latest_closed_rates_for_trading_client", + return_value=pd.DataFrame( + {"time": [1700000000, None], "close": [1.1, 1.2]}, + ), + ) + + with pytest.raises(ValueError, match=r"missing.*NaT.*timestamp"): + fetch_latest_closed_rates_indexed( + MagicMock(), + symbol="EURUSD", + granularity="M1", + count=2, + ) + + def test_drops_time_column_and_sets_index(self, mocker: MockerFixture) -> None: + """Test the returned DataFrame has the DatetimeIndex and no time column.""" + frame = pd.DataFrame( + { + "time": [1700000000, 1700003600], + "open": [1.0, 1.1], + "close": [1.1, 1.2], + }, + ) + mocker.patch( + "mt5cli.trading.fetch_latest_closed_rates_for_trading_client", + return_value=frame, + ) + + result = fetch_latest_closed_rates_indexed( + MagicMock(), + symbol="EURUSD", + granularity="M1", + count=2, + ) + + assert "time" not in result.columns + assert "open" in result.columns + assert "close" in result.columns + assert isinstance(result.index, pd.DatetimeIndex) diff --git a/uv.lock b/uv.lock index fddd2f6..09bb106 100644 --- a/uv.lock +++ b/uv.lock @@ -487,7 +487,7 @@ wheels = [ [[package]] name = "mt5cli" -version = "0.8.3" +version = "0.9.0" source = { editable = "." } dependencies = [ { name = "click" },