Skip to content

[networkx] Fix NodeDataView return type for string attribute in nodes(data=...) - #16695

Open
hrithiksaini99 wants to merge 1 commit into
python:mainfrom
hrithiksaini99:fix-networkx-node-data-view
Open

hrithiksaini99 wants to merge 1 commit into
python:mainfrom
hrithiksaini99:fix-networkx-node-data-view

Conversation

@hrithiksaini99

Copy link
Copy Markdown
Contributor

Fixes #16548

According to the NetworkX documentation and runtime behavior for Graph.nodes(data=...):

When called, if data is False, an iterator over nodes. Otherwise an iterator of 2-tuples (node, attribute value) where the attribute is specified in data. If data is True then the attribute becomes the entire data dictionary.

Previously, NodeView.__call__ and NodeView.data typed data: Literal[True] | str as returning NodeDataView[_Node, _NodeData, _EdgeData], which always yielded tuple[_Node, _NodeData] (the full dictionary) even when an attribute string was requested.

This PR:

  • Separates overloads for data: Literal[True] (returning NodeDataView[_Node, _NodeData]) and data: str (returning NodeDataView[_Node, _U], generic over the extracted attribute type).
  • Updates NodeDataView to be Generic[_Node, _D], yielding tuple[_Node, _D] on iteration and returning _D on item lookup (matching OutEdgeDataView).
  • Passes mypy tests across all supported Python versions (3.10-3.14).

…(data=...) (python#16548)

- Update NodeView.__call__ and NodeView.data overloads to return NodeDataView[_Node, _U] when data is a string
- Make NodeDataView generic over [_Node, _D] so __iter__ yields tuple[_Node, _D] and __getitem__ returns _D
- Fixes python#16548
@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉


@overload
def __call__(self, data: Literal[False] = False, default=None) -> Self: ...
def __call__(self, data: Literal[False] = False, default: Unused = None) -> Self: ...

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should try to remove the default argument for this branch to catch misuse of this API. If that doesn't work, try to use None as type argument.

def __call__(self, data: Literal[False] = False, default: Unused = None) -> Self: ...
@overload
def __call__(self, data: Literal[True] | str, default=None) -> NodeDataView[_Node, _NodeData, _EdgeData]: ...
def __call__(self, data: Literal[True], default: None = None) -> NodeDataView[_Node, _NodeData]: ...

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

Comment on lines +53 to +55
def data(self, data: Literal[False], default: Unused = None) -> Self: ...
@overload
def data(self, data: Literal[True] = True, default: None = None) -> NodeDataView[_Node, _NodeData]: ...

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above.

def data(self, data: str, default: _U | None = None) -> NodeDataView[_Node, _U]: ...

class NodeDataView(AbstractSet[_Node], Generic[_Node, _NodeData, _EdgeData]):
class NodeDataView(AbstractSet[_Node], Generic[_Node, _D]):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change _NodeData?

class NodeDataView(AbstractSet[_Node], Generic[_Node, _D]):
__slots__ = ("_nodes", "_data", "_default")
def __init__(self, nodedict: _NodeData, data: bool | str = False, default=None) -> None: ...
def __init__(self, nodedict, data: bool | str = False, default: object = None) -> None: ...

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why remove the annotation for nodedict?

Why use object for default? How is it used?

This branch has not been deployed

No deployments
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.

.nodes(data="...") on a networkx.Graph should return node data, not the whole dictionary

2 participants