Skip to content

fix: type safety and naming fixes in Python client.py#600

Merged
realfishsam merged 1 commit into
mainfrom
fix/563-449-452-456-496-client-py
May 24, 2026
Merged

fix: type safety and naming fixes in Python client.py#600
realfishsam merged 1 commit into
mainfrom
fix/563-449-452-456-496-client-py

Conversation

@realfishsam
Copy link
Copy Markdown
Contributor

Fixes #563
Fixes #449
Fixes #452
Fixes #456
Fixes #496

@realfishsam
Copy link
Copy Markdown
Contributor Author

PR Review: PASS (NOT VERIFIED)

What This Does

Multiple type safety and naming fixes in the Python client: (1) renames type param to order_type in create_order/build_order to avoid shadowing the Python builtin, (2) renames OrderBook.dt field to OrderBook.datetime in models.py and removes the 'dt': 'datetime' mapping from _SNAKE_TO_CAMEL, (3) adds _convert_params_to_camel() helper and applies it to 5 fetch methods (fetchMarkets, fetchMarketsPaginated, fetchEvents, fetchMarket, fetchEvent) so snake_case kwargs are converted to camelCase for the sidecar wire format, (4) adds concrete return types (MatchResult, PriceComparison, etc.) to 8 router methods that previously returned List[Any], (5) types watch_* callback parameters with Callable and return types with Dict[str, Any], (6) fixes list[str] to Optional[List[str]] on watch_address.

Blast Radius

  • sdks/python/pmxt/client.py -- parameter renames, param conversion, return types
  • sdks/python/pmxt/models.py -- OrderBook.dt renamed to OrderBook.datetime

Findings

  1. Breaking change: type renamed to order_type. All callers using exchange.create_order(type="limit") must update to exchange.create_order(order_type="limit"). This is the right move to avoid shadowing Python's type builtin, but it's a breaking change.
  2. Breaking change: OrderBook.dt renamed to OrderBook.datetime. Any code accessing orderbook.dt will break. However, 'dt': 'datetime' is removed from _SNAKE_TO_CAMEL, which means _auto_convert will now try to map the datetime field from the camelCase key datetime (which is identity). This is correct because the sidecar sends "datetime" as the JSON key. The old mapping was dt -> datetime in the mapping table so that the snake_case field dt would look up the camelCase key datetime in the raw dict. Now that the field IS named datetime, the identity mapping works.
  3. Merge conflict with PR 598. PR 598 makes side, type, amount keyword-only with no defaults. PR 600 renames type to order_type but keeps the defaults ("buy", "market", 0). The intended end state should be keyword-only order_type with no defaults. These PRs must be merged carefully.
  4. _convert_params_to_camel only converts top-level keys. This is documented in the docstring and consistent with the sidecar's expectations. Nested filter objects (e.g., filter={"text": "..."}) pass through as-is, which is correct because nested keys are already camelCase in the filter schema.
  5. New type imports (MatchResult, EventMatchResult, PriceComparison, ArbitrageOpportunity) from models.py. Verified these types exist in the Python models.py already. Import is correct.
  6. watch_address types param changed from list[str] = None to Optional[List[str]] = None. This is a Python 3.8 compatibility fix (bare list[str] requires 3.9+). Good fix.

PMXT Pipeline Check

  • Field propagation: N/A -- no new unified fields
  • OpenAPI sync: N/A -- SDK-only changes
  • Type safety: OK -- replaces Any with concrete types throughout

Semver Impact

major -- type to order_type rename and dt to datetime rename are both breaking changes to the public API

Risk

Merge conflict with PR 598 is certain (both touch create_order/build_order signatures). The _convert_params_to_camel helper is only applied to 5 methods -- other methods that accept params dicts (e.g., fetch_order_book, fetch_trades, fetch_ohlcv) still pass params as-is. Verify those methods don't need the same treatment, or document why they're excluded. No tests added for the camelCase conversion.

@realfishsam realfishsam merged commit 693d184 into main May 24, 2026
9 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment