networkx: Types for Graph properties are more general than required

[jenshnielsen]

The types introduced in #14569 are more general than required. As an example look at DiGraph.out_degree
This is now typed as def out_degree(self) -> int | OutDegreeView[_Node] | OutMultiDegreeView[_Node]: ...
This seems to be based on the documentation here that claims that an int can be returned. However looking at the examples and the documentation an int is only returned if a single node is given as an argument. This is a somewhat confusing statement as this is a property and therefor does not take any argument. Furthermore, looking at the source code an OutDegreeView is always the property. What is meant by the docstring is that the property returned which is always a OutDegreeView can return an int if that is called with a single node

import networkx as nx
G = nx.DiGraph()
nx.add_path(G, [0, 1, 2, 3])

out_view = G.out_degree  # this is always an `OutDegreeView`
G.out_degree(0)  # this returns an int
G.out_degree([0, 1, 2])

The same applies to in_degree and edges in the DiGraph class and probably similar in other graph classes.

Suggested solution remove int (and probably also ...MultiDegreeView from the return type of DiGraph.in_degree DiGraph.out_degree and DiGraph.degree and similar in other Graph classes.
It might also be worthwile improving the docs in networkx to clarify this. As a bonus the types for OutDegreeView could be improved such that it overloads on the type of nbunch to clarify that an int is returned if a single node is given and a view otherwise.

An example of the issues this creates can be found in microsoft/Qcodes#7401

[TomerGodinger]

I mentioned this here as well: #14499 (comment)
The addition of int seems to have been a mix-up, since the discussion has a point basically saying that int should probably not be in there.

[Avasam]

(and probably also ...MultiDegreeView from the return type of DiGraph.in_degree DiGraph.out_degree and DiGraph.degree and similar in other Graph classes.

Note that to respect the Liskov Substitution Principle, a base class' method should declare all of its subtypes possible return types for that method. Hence DiGraph.in_degree is declared to return a InMultiDegreeView because MultiDiGraph.in_degree can. (the same reasoning goes for out_degree)

InMultiDegreeView and InDegreeView do have a common base, DiDegreeView, but I believe that here the union is more helpful and closer to reality than using DiDegreeView as the return type.

This isn't immediately obvious if you don't remember the networkx Graph class tree by heart, hence I had added the helpful comment "# Include subtypes' possible return types" before it was removed by #14569 and #14509

# This has no type or runtime error, since MultiDiGraph is a subclass of DiGraph
# but you wouldn't get the expected return type !
from typing import Any
from networkx.classes.digraph import DiGraph
from networkx.classes.multidigraph import MultiDiGraph


def get_in_degree(graph: DiGraph[Any]):
    print(type(graph))
    print(type(graph.in_degree))
    # oops, this can actually return a InMultiDegreeView, not just InDegreeView !
    return graph.in_degree


get_in_degree(MultiDiGraph())

<class 'networkx.classes.multidigraph.MultiDiGraph'>
<class 'networkx.classes.reportviews.InMultiDegreeView'>

Though sometimes, in practice not respecting the LSP leads to less false-positives depending on common usage.
If, for example, typical networkx code working with a Graph or DiGraph wouldn't expect any subclass. Unfortunately there's no way in python's type system to say that you expect a specific class and not a subclass (invariant non-generic parameters?)

---

As for the int return type, I also agree from working on #14569 that it seems wrong. If someone feels like openning a PR to networkx to fix their doc it would surely help reduce confusion