code path returning None falsely detected in graphviz Dot.subgraph

[alexchandel]

Environment data

Language Server version: 2023.8.10
OS and version: macOS 13.4.1
Python version (& distribution if applicable, e.g. Anaconda): 3.11.4

Code Snippet

import graphviz

dot = graphviz.Digraph(comment='Foo', format='png')

with dot.subgraph(name=f'cluster_Foo') as c:  # Object of type "None" cannot be used with "with"
    # these all work fine:
    c.attr(label='foo')
    c.attr(style='filled', color='lightgrey')
    c.node_attr.update(style='filled', color='white')
    c.node('bar')

Source of Dot.subgraph() method.

Expected behavior

Pylance recognizes that when dot.subgraph() is not passed a graph keyword argument (i.e., graph=None), it always returns a contextmanager, never None.

Actual behavior

Pylance does not recognize this restriction.

Logs

N/A

[bschnurr]

you could try adding a type stub ".pyi" file and specify two different function signatures based on graph=None or not.
see https://microsoft.github.io/pyright/#/type-stubs?id=type-stub-files

or you could add a guard.

import graphviz

dot = graphviz.Digraph(comment='Foo', format='png')

subgraph = dot.subgraph(name=f'cluster_Foo')

assert subgraph is not None, "Subgraph creation failed."

with subgraph as c: 
    # these all work fine:
    c.attr(label='foo')
    c.attr(style='filled', color='lightgrey')
    c.node_attr.update(style='filled', color='white')
    c.node('bar')

[erictraut]

The graphviz library is untyped — i.e. it contains no static type information. In this case, pyright (the static code analyzer that underlies pylance) will do the best it can to infer the return type of a function like subgraph, but type inference relies on various heuristics that can fail. It can therefore lead to false positives.

If this is a library that you rely upon, I recommend reaching out to its maintainers and requesting that they add proper static type annotations to it. Maintainers of many popular libraries have done this over the past few years.

Another option is to create your own type stub that provides proper overload signatures for methods like this, but creating type stubs can be a lot of work. It also requires significant knowledge about the library you are stubbing. That's why it's best for the maintainers of a library to provide the type information as part of the library.

If you find that type checking is producing too many false positive errors caused by untyped libraries, you can set python.analysis.useLibraryCodeForTypes to false. This will cause pyright to infer the type of all symbols imported from untyped libraries as "Unknown". This will eliminate false positives, but it will also eliminate all completion suggestions and other type checking for these symbols.

You can also suppress all type checking diagnostics by setting python.analysis.typeCheckingMode to "off".

In any case, pyright (and pylance) are working as intended here. I don't consider this a bug.

[alexchandel]

@erictraut I assumed that pyright's heuristics allowed it to determine the return type based on inputs, in cases where an input's type determined the return type (like a simple dependent function). Is this not the case?

@bschnurr Is it possible to add stubs for just that function, without impairing pyright's heuristics in other cases?

[erictraut]

The primary way pyright determines the return type of a function is based on the return statements in that function's implementation. In cases where this fails to produce a known return type, pyright can perform a more advanced technique called "call-site return type inference", but this approach is very expensive (because it requires reanalyzing the entire function at every call site) and is therefore used in very narrow situations.

For more details, refer to this documentation.

As I said above, pyright is working as intended here. You'll need to use one of the above workarounds if you want to eliminate the type error that you're seeing in this case.

[alexchandel]

Is there an open feature request yet for adding more sophisticated/dependent return types? Something like _GeneratorContextManager[Dot] if graph is None else None, in my example.

Or (less powerfully) for inferring multiple type signatures depending on the inputs present, as @bschnurr suggested could be indicated with type stubs?

[erictraut]

We have a lot of experience with type inference. Pyright already implements return type inference techniques that are way more sophisticated than mypy or other Python type checkers — and even more sophisticated than TypeScript. More sophisticated return type inference (e.g. inferring overloads) wouldn't be a good solution. It would be very expensive (resulting in poor completion performance) and would work only in a small subset of cases.

The solution here is for library authors to provide correct type information for their libraries. As I mentioned above, many libraries have started to do this over the past few years. If graphviz is an important library for you, please log a feature request with the library's maintainer.