fix mypy errors after the 0.981 release #6652
Conversation
| @@ -199,7 +196,7 @@ def list_models(module: Optional[ModuleType] = None) -> List[str]: | |||
| return sorted(models) | |||
|
|
|||
|
|
|||
| def get_model_builder(name: str) -> Callable[..., M]: | |||
| def get_model_builder(name: str) -> Callable[..., nn.Module]: | |||
There was a problem hiding this comment.
Out of curiosity why do we need to change this one, and not all the other usages of M e.g. in register_model() above?
There was a problem hiding this comment.
TypeVar is used to create a "link" between parameters. For example I can do something like
def do_nothing(foo: M) -> M:
return fooThis tells mypy "whatever we have as input type, we return as output type as well". This is useful in case the function can handle multiple types, e.g.
T = TypeVar("T", int, float)
def add(a: T, b: T) -> T:
return a + bIn the case above, there is no input with the type M and so mypy complains that it can't figure out the M in the output.
Correct me if I'm wrong @datumbox, but I guess the annotation was intended to mean "we return any kind of model" here. Since M is defined as
vision/torchvision/models/_api.py
Line 168 in 06d9726
we can simply use nn.Module here.
| def _path_attribute_accessor(path: pathlib.Path, *, name: str) -> D: | ||
| return cast(D, _getattr_closure(path, attrs=name.split("."))) | ||
| def _path_attribute_accessor(path: pathlib.Path, *, name: str) -> Any: | ||
| return _getattr_closure(path, attrs=name.split(".")) |
There was a problem hiding this comment.
Why is this change needed?
Do you remember why we annotated the return with D in the first place?
There was a problem hiding this comment.
Why is this change needed?
Same as above. We no longer can use a TypeVar as return annotation if we don't have any input with the same type.
Do you remember why we annotated the return with
Din the first place?
IIRC, this is a remnant when this function was a local one inside
In there the annotation was correct and ok, since we have a D in the input parameters. At some point we factored them out since torchdata doesn't like local functions, and I probably forgot to fix this since mypy didn't complain.
|
BTW do the changes still pass with older versions of mypy? |
Most likely, but I'll check. The annotations were wrong to begin with, but older versions of Edit: I checked for |
Reviewed By: YosuaMichael Differential Revision: D39885431 fbshipit-source-id: a5b82afc1a54e78c50f36fadc0a2f9bd977edcb6
This fixes the ongoing
mypyCI failure caused by yesterdays0.981release