Skip to content

How important is the @_dispatchable decorator in networkx? #14462

Open

Description

@hamdanal

Can this decorator be removed?

One obvious advantage for having it is to mirror runtime. It has some disadvantages though:

  1. I can't for the life of me find a way to use it on overloaded functions. Indeed looking at the stubs, overloaded functions are stripped of this decorator
  2. VS Code with Pylance does not show the documentation of the function on hover only when the decorator is used

I find the overload case very important that it outweighs the benefits of matching runtime. I also think the editor hover issue compelling on its own but one could argue that this should be discussed with the Pylance team instead.

I wonder if we could just get rid of the decorator altogether. Am I missing other reasons for having the decorator?

Activity

  1. srittau commented on Jul 26, 2025

    @srittau
    Collaborator

    Without knowledge of networkx: For typeshed decorators are only relevant of they either have special meaning to type checkers (like @classmethod or @deprecated) or if they manipulate the signature somehow. Although in the latter case we might just use the modified signature instead. In some cases, keeping it as documentation might be worth it as well.

    I don't claim to understand the documentation of the @_dispatchable decorator, but to me it sounds like it's just an implementation detail. If it causes practical problems with annotations, I'd suggest we should just remove the decorators.

  2. hamdanal commented on Aug 2, 2025

    @hamdanal
    ContributorAuthor

    @Avasam looking at networkx change history I saw that you are the most involved maintainer in these stubs. I'd like to hear your opinion on removing the @_dispatchable decorator.

    For now, I've been leaving todo comments in places where the return type of a dispatchable function depends on an overload.

  3. Avasam commented on Aug 22, 2025

    @Avasam
    SponsorCollaborator

    Sorry for the late answer !

    @Avasam looking at networkx change history I saw that you are the most involved maintainer in these stubs.

    Mostly because I reviewed + followed-up on @rhelmot 's initial PR and removed the networkx stubs from microsoft/python-type-stubs#369 ^^


    One obvious advantage for having it is to mirror runtime.

    And to be more specific: it makes it easier and less surprising to stub. Especially if you don't know by heart what @_dispatchable does to the signature.

    However, one of the more recent networkx updates added a lot more @_dispatchable usages, which made

    I can't for the life of me find a way to use it on overloaded function

    a lot more noticeable.

    Am I missing other reasons for having the decorator?

    I'd say it massively reduces the amount of manual overloads we need to do in the stubs.

    See
    Image

    That's 781 locations where you'd have to update a single @_dispatchable for 2 overloads to be accurate. Or we indiscriminately add **backend_kwargs even for when backend=None and loose on type safety there, but avoids using any decorator. I guess this should be caught by stubtest ?


    Indeed overloading a function decorated with a callable who's __call__ method is itself overloaded is currently not supported by mypy or pyright.
    To quote @erictraut : microsoft/pyright#7167 (comment)

    I think your use of overload here is invalid, so I don't consider this a bug.

    An @overload decorator should be applied to a function or method, but you're applying it to a non-function object of type DecoratorClass.

    If you think this use case should be supported, the typing specification for @overload would need to be extended to define the rules for how it should be handled. You're certainly welcome to start a thread in the Python typing discussion forums to see if you could gather support for such an extension.

    And python/mypy#16840 (comment)

    [...] If we want to allow @overload to be used on callable objects — especially when those objects have overloaded signatures for __call__, then I think we'd need to extend the typing spec to accommodate this use case. As the typing spec is currently written, I wouldn't expect this to work.

    Although from a very quick glance I still don't see anything regarding callables classes or __call__ in https://typing.python.org/en/latest/spec/overload.html

    I personally think it would be beneficial if a type checker could automatically expand the overloads. In some bad cases it could explode to too many overloads real fast, but mypy already has a system of bailing out on too many overloads, so I don't see that as a blocker.
    I don't really have the motivation to start a discussion on the typing forums defending this idea though.


    1. VS Code with Pylance does not show the documentation of the function on hover only when the decorator is used

    That's not something I had considered.
    Pylance already does a bit of magic to "forward" or "passthrough" docstrings from the real Python file if the docs have none.

    one could argue that this should be discussed with the Pylance team instead.

    I think either way it think it's worth looking into, but as you said, on Pylance's side and on their issue tracker.

  4. rhelmot commented on Aug 22, 2025

    @rhelmot
    Contributor

    If you have a particular task for someone familiar with networkx and Python typing, I have some spare cycles.

  5. Avasam commented on Aug 22, 2025

    @Avasam
    SponsorCollaborator

    If you have a particular task for someone familiar with networkx and Python typing, I have some spare cycles.

    Atm mostly lamenting functions decorated with @_dispatchable that we need to overload on top of that ^^" And discussing if we should take a different approach or some other action.

    I think it's worth proposing to extend the typing spec, as Eric previously alluded to, to allow this use-case.

  6. Avasam commented on Aug 22, 2025

    @Avasam
    SponsorCollaborator

    The following almost works and would sidestep the double overload issue entirely, but you loose keyword params entirely.

    from collections.abc import Callable
    from typing import Any, Generic, TypeVar
    from typing_extensions import Self, TypeVarTuple, Unpack
    
    _R = TypeVar("_R")
    _Ts = TypeVarTuple("_Ts")
    
    class _dispatchable(Generic[Unpack[_Ts], _R]):
        def __new__(
            cls,
            func: Callable[_P, _R] | None = None,
            # ...
        ) -> Self: ...
        # Using @overload on __call__ also leads to issues where decorated methods can't also be overloaded
        # https://github.com/python/typeshed/issues/14462#issuecomment-3212917747
        # So we take the slight loss in type safety and IntelliSense for kwargs
        def __call__(self, *args: Unpack[_Ts], backend: str | None = None, **extra_backend_kwargs: Any) -> _R: ...

    It's a bit "pseudo-cody", but if the following would be possible:

    from collections.abc import Callable
    from typing import Any, Generic, TypeVar, TypedDict
    from typing_extensions import ParamSpec, Self
    
    _P = ParamSpec("_P")
    _R = TypeVar("_R")
    
    class _dispatchable(Generic[_P, _R]):
        def __new__(
            cls,
            func: Callable[_P, _R] | None = None,
            # ...
        ) -> Self: ...
        def __call__(self, *args: _P.args, backend: str, **kwargs: _P.kwargs, **extra_backend_kwargs: Any ) -> _R: ...
    
    # Or using the previously rejected keyword params concatenation, with a wishful use of TypedDict unpacking
    # https://peps.python.org/pep-0612/#concatenating-keyword-parameters
    
    class _BackendKWArgs(TypedDict, closed=False, total=False):
        backend: str
    
    class _dispatchable(Generic[_P, _R]):
        def __new__(
            cls,
            func: Callable[_P, _R] | None = None,
            # ...
        ) -> Self: ...
        __call__: Callable[Concatenate[_P, ("backend", str), Unpack[_BackendKWArgs]], _R]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions