-
-
Notifications
You must be signed in to change notification settings - Fork 30.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
incomplete signature with help function using typing #72176
Comments
the issue is that when calling help on a function annotated with typing, all the relevant information is lost, for example from typing import List, Any, Iterator, Tuple
def foo(data:List[Any]) -> Iterator[ Tuple[int,Any] ]:
...
when calling help on that >>> help(foo)
Help on function foo in module __main__: foo(data:typing.List) -> typing.Iterator
all the information is lost, the output should look like this >>> help(foo)
Help on function foo in module __main__: foo(data:List[Any]) -> Iterator[ Tuple[int, Any] ]:
where all the information that I put in the annotation is preserved and the typing.* are eliminated since they only add unnecessary noise while reporting this issue in the typing module (https://github.com/python/typing/issues/279) I was told that is a bug with the inspect module and that help may need modification. Thank for your time. |
It seems the output produced here is generated by inspect.signature(), which is called by pydoc in this case (in both versions of docroutine()). I don't know if the right thing to do is to change inspect.signature() here, or to change pydoc to use something else to format the argument list. |
More precisely, the issue is with inspect.formatannotation(), which overrides/ignores the repr if the annotation is an instance of type. Perhaps that should be changed to also check that __repr__ is type's repr. |
as that is the case, how about this as a solution: def formatannotation(annotation, base_module=None):
if isinstance(annotation, type):
if annotation.__module__ in ('builtins', base_module):
return annotation.__qualname__
elif annotation.__module__ in ('typing', base_module):
return repr(annotation).replace("typing.","")
return annotation.__module__+'.'+annotation.__qualname__
return repr(annotation) the same way that it check for builtins, do it for typing and clean up a little. With that change the result with the example is >>> help(foo)
Help on function foo in module __main__: foo(data:List[Any]) -> Iterator[Tuple[int, Any]]
|
That sounds a fine solution (except the elif should just test for There should be a unit test for this. |
It might be better to just change the if statement to 'if isinstance(annotation, type) and type(annotation).__repr__ is type.__repr__:'. That would make it fallback for any metaclass which overrides repr, instead of special-casing typing. That also ensures 'typing.' is still in the name, since these aren't builtins. |
I've lost you -- why don't you upload a patch? |
I think that removing the "typing." is for the best, with the example above >>> help(foo)
Help on function foo in module __main__: foo(data:typing.List[typing.Any]) -> typing.Iterator[typing.Tuple[int, typing.Any]]
leaving the "typing." produce result that I think are ugly and distracting, not only that, is unnecessary long to convey the same information that can be in a more neat way without it, and more so while more complicated/long the signature is. just compare the above with this >>> help(foo)
Help on function foo in module __main__: foo(data:List[Any]) -> Iterator[Tuple[int, Any]]:
which is a clear winner to me. Or perhaps alongside modifying inspect.formatannotation also change the repr in typing to exclude the >>> help(foo)
Help on function foo in module __main__: foo(data:~List[Any]) -> ~Iterator[~Tuple[int, ~Any]]:
which is a little weird but still neat |
Here is the patch according to the discussion (modifying inspect). I didn't change the rendering of docs for classes (neither stripped 'typing.' nor changed __bases__ to __orig_bases__). First, collections.abc.X are widely used as base classes, so that plain Mapping could be confused with collections.abc.Mapping. Second, seeing the actual runtime type-erased bases suggests that one should use isinstance() and issubclass() with those (not with, e.g., Mapping[int, str], the latter will raise TypeError). |
Hm, I actually like the original proposal better. Perhaps collections.abc.Mapping is more common than typing.Mapping, but is it more common *in function annotations*? I don't think so. Also, I like showing e.g. Iterator[Tuple[int, Any]] rather than just Iterator. This is documentation we're talking about, and the parameter types are very useful as documentation. (However, abbreviating List[Any] as List is fine, since they mean the same thing.) |
For function annotations I did as originally proposed. In my previous comment I was talking about documentation for classes. For example: class C(Generic[T], Mapping[int, str]): ...
pydoc.render_doc(C) will show "class C(typing.Mapping)". while for function annotations typing is indeed much more common so that pydoc.render_doc(foo) will show foo(data: List[Any]) -> Iterator[Tuple[int, Any]] |
OK, sounds good then. I guess most of the work was in typing.py, not in inspect. :-) |
Actually, for classes, it is probably worth adding a separate section "Generic type info" that will render information using __orig_bases__, __parameters__, and __args__. At the same time the "header" will be the same as now, listing runtime __bases__. What do you think about this? Should I open a separate issue? |
New changeset dc030d15f80d by Guido van Rossum in branch '3.5': New changeset 3937502c149d by Guido van Rossum in branch '3.6': New changeset 62127e60e7b0 by Guido van Rossum in branch 'default': |
Honestly I think pydoc is already too verbose. It would be better if the class header looked more like what was written in the source code -- that is the most compact way to render it. I say open a separate issue since this issue is about functions. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: