Fix sidebar #602
Fix sidebar #602
Conversation
Codecov Report
@@ Coverage Diff @@
## master #602 +/- ##
==========================================
+ Coverage 91.13% 91.16% +0.02%
==========================================
Files 46 46
Lines 7839 7840 +1
Branches 1701 1701
==========================================
+ Hits 7144 7147 +3
+ Misses 434 433 -1
+ Partials 261 260 -1
Continue to review full report at Codecov.
|
pydoctor/templatewriter/util.py
Outdated
| for inherited_via,attrs in inherited_members(cls): | ||
| if len(inherited_via)>1: | ||
| children.extend(attrs) |
There was a problem hiding this comment.
This is the actual fix!
| @@ -166,7 +172,7 @@ def _children(self, inherited: bool = False) -> Optional[List[Documentable]]: | |||
| return sorted((o for o in util.list_inherited_members(self.ob) if o.isVisible), | |||
| key=util.objects_order) | |||
| else: | |||
| return None | |||
| assert False, "Use inherited=True only with Class instances" | |||
There was a problem hiding this comment.
I appreciate trying to keep the diff small, but assertions in the negative path like this are somewhat confusing to read; it seems like there's going to be a path through at first and then there isn't. When reading the assertion in a traceback it's less clear what's being asserted. Instead, I'd do:
assert isinstance(self.ob, Class), "Use inherited=True only with Class instances"
return sorted((o for o in util.list_inherited_members(self.ob) if o.isVisible), key=util.objects_order)Sorry that's not a suggestion, but it modifies lines outside the diff…
There was a problem hiding this comment.
Thanks, I've applies your suggestion.
|
|
||
| if isinstance(self.ob, Class): |
There was a problem hiding this comment.
Longer-term question: could this be added as a method to Documentable so we can just call it on self.ob rather than dispatching on type?
There was a problem hiding this comment.
You mean the _children() method?
I’ts using processing code that is not currently in the model, in the util.py file.
But I agree it could be moved in the model.
We could introduce a method as follow:
def children(inherited:bool=False, ofclass:Optional[Type[Documentable]]=None) -> Iterator[Documentable]
Fixes #601