massive-com / massive-com/client-python

Inconsistent typing for WebSocketMessage and WebSocketClient.run() callback in v2.8.0

Offen
#1,022 0 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen

Dieses Issue hat noch niemand übernommen.

bug
Vorherrschende Sprache
Python
Sterne
1.5k
Forks
362
PR-Merge-Kennzahlen
Keine gemergten PRs in 30 T.

Beschreibung

Hi Massive team,

I noticed an inconsistency in the websocket typing in Massive Python SDK v2.8.0.

WebSocketMessage is currently defined as a NewType over a list of parsed event models:

WebSocketMessage = NewType(
    "WebSocketMessage",
    List[
        Union[
            EquityAgg,
            CurrencyAgg,
            EquityTrade,
            ...
        ]
    ],
)

However, WebSocketClient.run() is typed as accepting a callback of:

Callable[[List[WebSocketMessage]], None]

This effectively makes the callback parameter type a nested list-like structure:
List[WebSocketMessage], while WebSocketMessage itself already represents a list
of parsed websocket events.

This causes type checkers such as Pyright/Pylance to reject handlers like this:

def handle_msg(messages: WebSocketMessage) -> None:
    for message in messages:
        if isinstance(message, EquityAgg):
            ...

with an error similar to:

Argument of type "(messages: WebSocketMessage) -> None" cannot be assigned to parameter "handle_msg"
Type "(messages: WebSocketMessage) -> None" is not assignable to type "(List[WebSocketMessage]) -> None"

There also seems to be a related inconsistency in the parser:

def parse_single(...) -> Optional[WebSocketMessage]:
    parsed = model_class.from_dict(data)
    return cast(WebSocketMessage, parsed)

At runtime, parsed is a single event object such as EquityAgg, not a list. Then
parse() returns List[WebSocketMessage].

So there appear to be two possible fixes:

  1. If WebSocketMessage is intended to mean a single parsed event, redefine it as
    a union of event model types, not a list.
  2. If WebSocketMessage is intended to mean a batch of parsed events, then run()
    should accept Callable[[WebSocketMessage], None], and parse_single() should
    not cast individual event objects to WebSocketMessage.

Based on the current runtime behavior, option 1 seems more natural:

WebSocketMessage = NewType(
    "WebSocketMessage",
    Union[
        EquityAgg,
        CurrencyAgg,
        EquityTrade,
        ...
    ],
)

or alternatively, using a type alias instead of NewType:

WebSocketMessage = Union[
    EquityAgg,
    CurrencyAgg,
    EquityTrade,
    ...
]

Then these signatures would make sense:

def parse_single(...) -> Optional[WebSocketMessage]: ...
def parse(...) -> List[WebSocketMessage]: ...

def run(
    self,
    handle_msg: Callable[[List[WebSocketMessage]], None] | Callable[[str | bytes], None],
    close_timeout: int = 1,
    **kwargs: Any,
) -> None: ...

One smaller typing improvement: run() currently leaves **kwargs unknown for
Pyright/Pylance. Annotating it as **kwargs: Any and adding -> None would avoid
"partially unknown" warnings.

Thanks.

Beitragsleitfaden

Für dieses Repository ist kein Beitragsleitfaden indexiert

Erste Schritte

  1. Lies das ganze Issue und danach den Beitragsleitfaden des Projekts.
  2. Schreib ins Issue, dass du es übernimmst — das erspart doppelte Arbeit.
  3. Forke das Repository und arbeite in einem Branch.
  4. Öffne einen Pull Request, der die Issue-Nummer nennt.

Rechercherichtung

Beginne mit den im Issue erwähnten Definitionen von WebSocketMessage, WebSocketClient.run(), parse_single() und parse(), und untersuche anschließend deren Laufzeitwerte und Typsignaturen. Bestätige, ob WebSocketMessage ein einzelnes geparstes Ereignis oder einen Stapel darstellt, richte die Callback- und Parser-Annotationen entsprechend aus und füge die vorgeschlagenen Annotationen für run() ein, damit Pyright/Pylance einen korrekt typisierten Handler akzeptiert.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Bewertung

Tech-Stack
python
Bereich
api, networking
Issue-Typ
Bug
Schwierigkeit
3/5
Geschätzter Aufwand
1-2 Tage
Aktivitätsstatus
Ruhig
Klarheit
Größtenteils klar
Anfängerfreundlichkeit
55/100

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.