Skip to content

fix: export Python websocket return types#511

Merged
realfishsam merged 2 commits into
pmxt-dev:mainfrom
iccccccccccccc:fix-python-init-export-ws-types
May 24, 2026
Merged

fix: export Python websocket return types#511
realfishsam merged 2 commits into
pmxt-dev:mainfrom
iccccccccccccc:fix-python-init-export-ws-types

Conversation

@iccccccccccccc

Copy link
Copy Markdown
Contributor

Fixes #461

Changes

  • export FirehoseEvent and SubscribedAddressSnapshot from the Python package entry point
  • add a focused regression test for the Python public export list
  • include the generator drift fixes from fix: initialize Baozi order book resolver queues #509 so current main passes the exchange/generator checks

Verification

  • python -m pytest sdks/python/tests/test_public_exports.py
  • python -m py_compile sdks/python/pmxt/__init__.py sdks/python/pmxt/models.py sdks/python/tests/test_public_exports.py
  • node core/scripts/check-exchange-drift.js
  • node sdks/typescript/scripts/generate-client-methods.js; git diff --exit-code sdks/typescript/pmxt/client.ts
  • node sdks/python/scripts/generate-client-methods.js; git diff --exit-code sdks/python/pmxt/client.py
  • npx tsc -p core/tsconfig.json --noEmit
  • git diff --check

@realfishsam

Copy link
Copy Markdown
Contributor

PR Review: PASS (NOT VERIFIED)

What This Does

Fixes #461 by exporting FirehoseEvent and SubscribedAddressSnapshot from the Python SDK's __init__.py (both the import block and __all__). Additionally bundles two generator drift fixes from #509: (1) renames the Python class Polymarket_us to PolymarketUS to match the TypeScript SDK convention, and (2) adds a fetchOrderBook special case to both SDK generators so regeneration preserves the hand-written array-return handling.

Blast Radius

  • Python SDK: __init__.py (exports), _exchanges.py (class rename), generator script
  • TypeScript SDK: generator script only (no runtime change to client.ts)
  • Core: generate-python-exchanges.js (toClassName logic + CRLF regex)
  • Docs: Both API_REFERENCE.md files regenerated (generated output, not hand-edits)

Test Results

  • Build: NOT RUN (bash permissions restricted in review environment)
  • Unit tests: NOT RUN

Findings

  1. Polymarket_us -> PolymarketUS is a breaking change for Python users (sdks/python/pmxt/_exchanges.py:368, sdks/python/pmxt/__init__.py:20,146). Any Python user writing pmxt.Polymarket_us() will get an ImportError after upgrading. The TypeScript SDK already uses PolymarketUS, so this aligns the two SDKs, but there is no deprecation alias (e.g., Polymarket_us = PolymarketUS) to ease migration. Recommend adding a deprecation alias or documenting the rename in the changelog.

  2. fetchOrderBook generator special case duplicates existing hand-written code (sdks/python/scripts/generate-client-methods.js:390-424, sdks/typescript/scripts/generate-client-methods.js:384-412). The current client.py and client.ts on main already contain the correct hand-written fetchOrderBook with array-return handling. The PR adds the same logic as a hardcoded special case in both generators so that running regeneration does not clobber it. This is correct and necessary -- the BaseExchange.ts declares the return type as Promise<OrderBook> (single), but the runtime can return OrderBook[] when since+until are provided, so the generic generator would produce broken code. However, the right long-term fix would be to update the BaseExchange.ts return type to Promise<OrderBook | OrderBook[]> and teach the generators about union return types.

  3. toClassName change has a special case only for 'us' (core/scripts/generate-python-exchanges.js:33). The new logic part.toLowerCase() === 'us' ? 'US' hardcodes a single abbreviation. If future exchanges have other abbreviations (e.g., uk, eu), each would need another special case. This is fine for now but worth noting.

  4. CRLF regex fix is defensive and correct (core/scripts/generate-python-exchanges.js:247). The \r?\n change makes the __all__ regex work on Windows line endings. No issue.

  5. Test test_public_exports.py is well-designed (sdks/python/tests/test_public_exports.py). Uses AST parsing to verify imports and __all__ entries without importing the package (which would require the sidecar). Good regression test.

  6. API_REFERENCE.md changes are consistent with regenerated output -- the testDummyMethod -> close rename reflects that testDummyMethod was removed from BaseExchange.ts and close() was added. The FetchOrderBookParams type documentation was added. These are generated files.

PMXT Pipeline Check

  • Field propagation (3-layer): OK -- FirehoseEvent and SubscribedAddressSnapshot were already in models.py and client.py imports; only __init__.py was missing them
  • OpenAPI sync: N/A -- no OpenAPI schema changes
  • Type safety: OK -- no any types introduced, no non-null assertions

Semver Impact

minor -- The Polymarket_us -> PolymarketUS rename is a breaking change for Python users, but polymarket_us is a relatively new exchange and the rename aligns with the established TypeScript convention. If any users depend on the old name, a deprecation alias should be added (which would make this a patch).

Risk

  • Tests were not run due to environment restrictions. The PR's verification steps (listed in the PR body) should be run by CI.
  • The Polymarket_us -> PolymarketUS rename has no backwards-compatibility alias. Users upgrading will get a hard ImportError with no guidance.

@iccccccccccccc

Copy link
Copy Markdown
Contributor Author

Good catch on the Python class rename. I added a backwards-compatible Polymarket_us = PolymarketUS alias, kept it exported from pmxt.all, and updated the Python exchange generator so a future regen preserves the alias instead of dropping it. I also extended est_public_exports.py to cover that alias and reran python -m pytest sdks/python/tests/test_public_exports.py.

@iccccccccccccc iccccccccccccc force-pushed the fix-python-init-export-ws-types branch from 5162176 to 01094d5 Compare May 24, 2026 14:14
@realfishsam realfishsam merged commit 5f3a811 into pmxt-dev:main May 24, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SubscribedAddressSnapshot and FirehoseEvent not exported from Python __init__.py

2 participants