Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
t-ded opened this issue Jul 5, 2024 · 1 comment · Fixed by #1247
Closed

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

t-ded opened this issue Jul 5, 2024 · 1 comment · Fixed by #1247
Assignees
Labels
bug Something isn't working

Comments

@t-ded
Copy link

t-ded commented Jul 5, 2024

Information

  • rustworkx version: 0.15.1
  • Python version: 3.11.5
  • Rust version: Rustworkx installed via pip
  • Operating system: macOS Sonoma 14.5

What is the current behavior?

Both graph and digraph _node_link_json serializers show expected callable output as str type.

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

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

let attr_callable = |attrs: &PyObject, obj: &PyObject| -> PyResult<BTreeMap<String, String>> {
let res = attrs.call1(py, (obj,))?;
res.extract(py)
};

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

@t-ded t-ded added the bug Something isn't working label Jul 5, 2024
@IvanIsCoding
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants