Qiskit / rustworkx

A high performance Python graph library implemented in Rust.
https://www.rustworkx.org
Apache License 2.0
966 stars 140 forks source link

Incorrect Python stubs typing for node/edge attrs parameters in json serializers #1243

Open t-ded opened 3 days ago

t-ded commented 3 days ago

Information

What is the current behavior?

Both graph and digraph _node_link_json serializers show expected callable output as str type. https://github.com/Qiskit/rustworkx/blob/553bff1823a30293c82fb811f4457b094700a728/rustworkx/rustworkx.pyi#L632-L647

Meanwhile, the docs state 'is expected to return a dictionary of string keys to string values representing the data payload' (same as for the graph_attrs parameter, for which the typing corresponds with the documentation).

From the rust function, the dictionary is also expected https://github.com/Qiskit/rustworkx/blob/553bff1823a30293c82fb811f4457b094700a728/src/json/node_link_data.rs#L125-L128

On inserting a callable return a str datatype, this throws an error: E TypeError: 'str' object cannot be converted to 'PyDict' No error is thrown on return a dict

What is the expected behavior?

def digraph_node_link_json(
    graph: PyDiGraph[_S, _T],
    /,
    path: str | None = ...,
    graph_attrs: Callable[[Any], dict[str, str]] | None = ...,
    node_attrs: Callable[[_S], dict[str, str]] | None = ...,
    edge_attrs: Callable[[_T], dict[str, str]] | None = ...,
) -> str | None: ...

def graph_node_link_json(
    graph: PyGraph[_S, _T],
    /,
    path: str | None = ...,
    graph_attrs: Callable[[Any], dict[str, str]] | None = ...,
    node_attrs: Callable[[_S], dict[str, str]] | None = ...,
    edge_attrs: Callable[[_T], dict[str, str]] | None = ...,
) -> str | None: ...

Steps to reproduce the problem

Inserting node_attrs=lambda x: 'a', which corresponds to the Callable[[_S], str] | None type

IvanIsCoding commented 2 days ago

Our type annotations are added manually, so I think you just found a mismatch in the annotations! I might bundle the fix of this with #1242 for a 0.15.2 release. And of course it will be fixed by 0.16 for sure