Added typing annotations to models/video#4229
Conversation
|
Same here @fmassa, the failed test looks unrelated, ready for review 👌 |
datumbox
left a comment
There was a problem hiding this comment.
Thanks for the PR.
I spotted few cases where the intended type might be different from what you added. Unfortunately our code was not too clear either in this case. I flagged some of them, let me know your thoughts.
| zero_init_residual=False): | ||
| def __init__( | ||
| self, | ||
| block: Type[Union[BasicBlock, Bottleneck]], |
There was a problem hiding this comment.
I believe block should be an nn.Module. The intention is to allow users pass their own custom blocks
There was a problem hiding this comment.
Alright, but apart from nn.Module default class, it needs an expansion attribute
There was a problem hiding this comment.
You are right, leave as is. :)
There was a problem hiding this comment.
| block: Type[Union[BasicBlock, Bottleneck]], | |
| block: Callable[..., Union[BasicBlock, Bottleneck]], |
|
@datumbox Any clue about the reason of this failure? |
datumbox
left a comment
There was a problem hiding this comment.
Apparently
./torchvision/models/video/resnet.py:223 in private method `__init__`:
D417: Missing argument descriptions in the docstring (argument(s) conv_makers are missing descriptions in '__init__' docstring)
Try these:
| zero_init_residual=False): | ||
| def __init__( | ||
| self, | ||
| block: Type[Union[BasicBlock, Bottleneck]], |
There was a problem hiding this comment.
| block: Type[Union[BasicBlock, Bottleneck]], | |
| block: Callable[..., Union[BasicBlock, Bottleneck]], |
Summary: * style: Added typing to models/video * style: Fixed typing * style: Fixed typing * style: Fixed typing * refactor: Removed default value for stem * docs: Fixed docstring of VideoResNet * style: Refactored typing * docs: Fixed docstring * style: Fixed typing * docs: Specified docstring * typing: Fixed tying * docs: Fixed docstring * Undoing change. Reviewed By: fmassa Differential Revision: D30525888 fbshipit-source-id: 0015be22677c06f07d5d8f3c2e46fe05c2a455e4
Following up on #2025, this PR adds missing typing annotations in
models/video.Any feedback is welcome!
cc @datumbox