Enabled typing check for densenet

[frgfm]

Following up on #3029, this PR introduces the following modifications:

• enabled typing check for torchvision.models.densenet
• added proper typing exceptions when needed thanks to Support mypy ignore annotation with particular rule specified pytorch#51675

Please note that there is still an error related to a typing issue in torch (adaptive pooling), I'll check with them if this has been solved.

Any feedback is welcome!

cc @pmeier

[frgfm]

According to https://github.com/pytorch/pytorch/blob/master/torch/nn/functional.pyi.in#L89 and https://github.com/pytorch/pytorch/blob/master/torch/nn/common_types.py#L29, I believe the typing error about adaptive_avg_pool2d has already been solved on the master branch of pytorch 👌

[pmeier]

@frgfm the failing tests are related:

RuntimeError: expected type comment but found 'def' here:
def forward(self, input: Tensor) -> Tensor:  # type: ignore[no-redef] # noqa: F811
~~~ <--- HERE
self = <test_models.ModelTester testMethod=test_densenet121_cpu>
model_name = 'densenet121', dev = device(type='cpu')

    def do_test(self, model_name=model_name, dev=dev):
        input_shape = (1, 3, 224, 224)
        if model_name in ['inception_v3']:
            input_shape = (1, 3, 299, 299)
>       self._test_classification_model(model_name, input_shape, dev)

test/test_models.py:440: 

This seems similar to what we had before. I think pytorch/pytorch#51675 only fixed this if the type: ignore[...] is at the end of the line. Could you test this?

[frgfm]

@pmeier Mmmh weird, locally I'm not getting this error. It's probably because I'm running not running pytorch from source. And for the mypy test, I only have those with my proposal:

torchvision/models/mobilenetv3.py:31: error: Argument 2 to "adaptive_avg_pool2d" has incompatible
type "int"; expected "Union[Size, List[int], Tuple[int, ...]]"  [arg-type]
            scale = F.adaptive_avg_pool2d(input, 1)
                                                 ^
torchvision/__init__.py:16: error: Skipping analyzing 'torchvision.version': found module but no
type hints or library stubs  [import]
        from .version import __version__  # noqa: F401
    ^
torchvision/__init__.py:16: note: See https://mypy.readthedocs.io/en/latest/running_mypy.html#missing-imports

and if I exchanged the order with the noqa, I'm getting the old errors back (and I tried single/double spacing and tabs to separate it from the noqa).

I'm not sure I understand the CI error though: is it suggesting to put the type comment after the def line? I'll try to change the order anyway to see if this does it

[pmeier]

It's probably because I'm running not running pytorch from source

What version of torch are you running? It should be either from source or the nightly build. I suggest you use the latter for convenience if you don't simultaneously work on PyTorch core.

I'm not sure I understand the CI error though

Right now the unittests succeed, but mypy fails though. I'll investigate and get back to you.

[frgfm]

What version of torch are you running? It should be either from source or the nightly build. I suggest you use the latter for convenience if you don't simultaneously work on PyTorch core.

You're right, I'm running the latest stable version 1.7.1 at the moment

Right now the unittests succeed, but mypy fails though. I'll investigate and get back to you.

Thanks! Considering pytorch/pytorch#51675 was merged only 7 days ago into master, I'm not sure whether torchvision CI uses a source build of pytorch or a nightly release, but we might have to wait a bit to run those tests successfully 🤷‍♂️

[pmeier]

I'm not sure whether torchvision CI uses a source build of pytorch or a nightly release, but we might have to wait a bit to run those tests successfully

As the name implies, the nightly build is done once per day. Thus if this was merged two or more days ago, we should be fine.

---

The problem at hand is that now torchscript requires the type: ignore comments to be at the end of the line, whereas mypy does not work if the flake8 directive (# noqa ...) comes before the type: ignore comments. Thus, right now we can only satisfy one of them. I've opened pytorch/pytorch#52271 to overcome this.

If this goes through you can revert your latest change. In a quick search I couldn't find if this is intended behavior of mypy or a known bug. If you want to push this forward you can also raise this issue to them.

[frgfm]

Hi @pmeier,

From what I can see in the issue you opened, the problem is still ongoing?

[pmeier]

@frgfm At least it is not fixed yet. Let me ping someone there to get an update.

[frgfm]

@frgfm At least it is not fixed yet. Let me ping someone there to get an update.

FYI I just pinged the assignee in the corresponding issue, let's hope we get an update soon 🤞

[pytorch-bot]

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/vision/3395

• 📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

❌ 2 New Failures, 1 Unrelated Failure

As of commit a0c4a85 with merge base 75046bd ():

NEW FAILURES - The following jobs have failed:

• python-source-and-configs / linux-job (gh)
• python-types / linux-job (gh)

FLAKY - The following job failed but was likely due to flakiness present on trunk:

• unittests-macos (3.8, macos-m1-12) / macos-job (gh)

This comment was automatically generated by Dr. CI and updates every 15 minutes.

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (main@75046bd). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head 221d615 differs from pull request most recent head a0c4a85. Consider uploading reports for the commit a0c4a85 to get more accurate results

@@           Coverage Diff           @@
##             main    #3395   +/-   ##
=======================================
  Coverage        ?   74.82%           
=======================================
  Files           ?      105           
  Lines           ?     9719           
  Branches        ?     1562           
=======================================
  Hits            ?     7272           
  Misses          ?     1960           
  Partials        ?      487           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[frgfm]

Quick update here: the only problem left is the inability to ignore type-redef mentioned above. Keen to hear if you have any idea!