Add return type information only for functions #220
Conversation
Codecov Report
@@ Coverage Diff @@
## master #220 +/- ##
==========================================
+ Coverage 79.56% 79.66% +0.09%
==========================================
Files 23 23
Lines 3616 3619 +3
Branches 786 788 +2
==========================================
+ Hits 2877 2883 +6
+ Misses 512 511 -1
+ Partials 227 225 -2
Continue to review full report at Codecov.
|
pydoctor/test/__init__.py
Outdated
| class CapLog: | ||
| records: Sequence[LogRecord] | ||
|
|
||
| class CaptureResult(NamedTuple): | ||
| out: str | ||
| err: str | ||
|
|
||
| class CapSys: | ||
| def readouterr(self) -> CaptureResult: ... |
There was a problem hiding this comment.
might be better to use Protocols here, rather than concrete types - as that's what the pytest fixture types in pytest. will be.
otherwise you can use the exported types in _pytest.logging.LogCaptureFixture
| class CapLog: | |
| records: Sequence[LogRecord] | |
| class CaptureResult(NamedTuple): | |
| out: str | |
| err: str | |
| class CapSys: | |
| def readouterr(self) -> CaptureResult: ... | |
| from typing import TYPE_CHECKING | |
| if TYPE_CHECKING: | |
| from typing_extensions import Protocol | |
| class CapLog(Protocol): | |
| records: Sequence[LogRecord] | |
| class CaptureResult(Protocol): | |
| out: str | |
| err: str | |
| class CapSys(Protocol): | |
| def readouterr(self) -> CaptureResult: ... | |
| else: | |
| CapLog = CaptureResult = CapSys = None |
There was a problem hiding this comment.
That's probably a good idea, for #219 though.
There were 3 copies of this function already, as inner functions named format(), and I'm planning to add more test cases that use it.
Currently this test fails because too much output is produced.
The return type information was added for all Documentables, which is both unnecessary and leads to incorrect output. Also do not redefine 'annotations' with a different type; use a separate name instead.
The HTML5 spec specifies that a <table> element must contain "either zero or more <tbody> elements or one or more <tr> elements". Besides spec compliance, the HTML output is just smaller and cleaner without the empty table.
mthuurne
force-pushed
the
rettype-only-functions
branch
from
a9fcd5c
to
5e9c8e5
Compare
| } | ||
| ret_type = formatted_annotations.pop('return', None) | ||
| ret_value = cls(obj, formatted_annotations) | ||
| if ret_type is not None: |
There was a problem hiding this comment.
Instead of doing a None sentinel, couldn't we do it with exceptions?
try:
ret_type = formatted_annotations.pop("return")
except KeyError:
pass
else:
return_type = FieldDesc()
return_type.body = ret_type
ret_value.handle_returntype(return_type)
There was a problem hiding this comment.
Since ret_value is needed whether the return annotation exists or not, its creation must happen on both flows. But if it is moved before the pop(), then the return annotation will be mixed into the parameter annotations, which isn't how FieldHandler is designed. So we'd either have to duplicate the ret_value initialization or put ret_type = None in the except: clause and keep the if ret_type is not None:, neither of which is appealing.
Actually ret_value is a poorly chosen name here: it's the return value of the method, but has nothing to do with the return statement, unlike ret_type. I'll rename it to handler.
The name `ret_value` is confusing next to `ret_type`, since the former is the return value of from_ast_annotations(), while the latter is the type annotation for the return type of the introspected function.
I noticed a stray "Returns" at the end of module descriptions when using pydoctor's
masterbranch, but not with the 20.7.1 release. So this is a regression, probably introduced as part of #209.The problem is that an empty
FieldDescis assigned toreturn_desc, which ends up in the HTML output as a return value because it evaluates toTrueeven when empty. Rather than trying to define exactly how empty aFieldDescmust be to be consideredFalse, I decided it was clearer to leavereturn_descon its default value ofNoneforDocumentablesother than functions.