Open rmorshea opened 6 months ago
I don't think there's a "correct" answer here. I can point to cases where either choice is correct. The current behavior is at least internally consistent. If a callable object has a __call__
method with a docstring, that docstring takes precedence over any docstring that is captured by a ParamSpec used within that __call__
method's signature.
You can omit the docstring for the __call__
method if you don't want it to take precedence. Pyright/pylance will then use the docstring captured by the ParamSpec.
For reference, I've updated the description to note that Jedi appears to implement the behavior I'd like given the example I provided.
You can omit the docstring for the
__call__
method if you don't want it to take precedence.
Perhaps I'm not on the latest version, but in my example, the docstring is not displayed even though RequiresOptions.__call__
lacks a docstring. Removing the docstring from RequiresOptions
itself doesn't change that either.
I'd definitely be satisfied with this behavior though. For whatever reason I'm just not seeing it in my example.
I see what you mean. The docstring does appear in the signature help (when typing the arguments for the foo
call), but it doesn't appear when hovering over foo
. That's a current limitation of the hover provider — the language server module that provides the hover text. I agree that it would make sense for the hover provider to include the "adopted" decorator in this case in the same way that the signature help provider does.
Here's the signature help for foo
. You can see that the "adopted" docstring does appear here.
In summary, I think the pyright type analyzer is doing the right thing here and preserving the docstring, but the hover provider (which is owned by the pylance team) is not using this information when the identifier is an instance of a callable object (i.e. an instance of a class that has a __call__
method). So I think this is a feature request for the Pylance team to add such support.
Great. Thanks for digging into the issue. I Look forward to having it fixed!
when we rework how we do doc string this sprint, we should make sure all features that use doc string use same code to make sure we have consistent behavior regardless where it is shown to users.
currently, we have slightly different code depends on each features.
Current Behavior
When implementing decorators it's often convenient to use a
Protocol
andParamSpec
to annotate the type they return. However, using aProtocol
causes Pylance to hide the docstring of the decorated function even though, in most cases where aProtocol
is generalized by aParamSpec
, it would make sense to display the docstring of the function theParamSpec
came from.Consider the example below:
Here, Pylance just shows the type of
foo
. One could argue that this is sensible given that the decorator changed the type offoo
. That might imply that the foo's docstring wouldn't necessarily be meaningful anymore. However, if that's true, Pylance isn't consistent in its behavior since, if we change this decorator to useConcatenate
instead of aProtocol
, the docstring is preserved:Expected Behavior
When Pylance encounters decorators of the form:
Pylance should preserve the docstrings of the functions it decorates. Further, the details of
__call__
should not matter beyond the fact that it uses theParamSpec
. So for example, any of the following should still preserve docstrings:Given the same example above Jedi chooses to display the docstring of the decorated function:
Open Questions
It's a little unclear whether any
Protocol
instances that are generalized by a function'sParamSpec
should adopt the docstring of that function. Perhaps only aProtocol
that implements a__call__
that uses theParamSpec
should have this behavior.For example, it might not make sense to copy the docstring in a case like:
It's not particularly clear what the best thing to do in this situation is.