Annotation Problem *.pyi

[Roffild]

🚀 Feature

It is difficult to control the validity of function arguments when they are in different files.
Very often the information in *.pyi does not match *.py.

pytorch/torch/utils/data/dataloader.pyi

Line 35 in fcd1354

class _DataLoaderIter:

I suggest using Python 2 Annotations:

def optional_unwrap (self, x, y):
   # type: (Optional [int], Optional [int]) -> int

You can expand this format to indicate in which file to place annotations.
After that, all *.pyi can be completely assembled in one script. Information will always be up to date.

cc @ezyang @gchanan @zou3519

[vincentqb]

(Python 2 support will be dropped #23795)

[malmaud]

I think everyone would be onboard to move to inline type annotations once Python 2 support is dropped. The current preference was to use stubs instead of comment-based annotations.

Was there a specific incorrect stub?

[vincentqb]

We'll wait for Python 2 EOL, and move all annotation inline using Python 3 syntax.

[ngoldbaum]

Was there a specific incorrect stub?

It looks like _DataLoaderIter is now called _BaseDataLoaderIter and needs to be renamed and updated in the stubs (and possibly the various subclasses need to be added as well).

More generically, there should probably be a test that checks that the stubs agree with the actual implementation, at least so long as the mypy type information is in a separate file from the implementation.

I'll go ahead and put in a pull request this afternoon that hopefully fixes the specific issue with the type stubs for _BaseDataLoaderIter, this is my first pytorch issue so bear with me :)

[malmaud]

Thanks! There are some tests for the most important stubs but they are far from comprehensive at the moment.

…

On Mon, Sep 30, 2019 at 3:52 PM Nathan Goldbaum ***@***.***> wrote: Was there a specific incorrect stub? It looks like _DataLoaderIter is now called _BaseDataLoaderIter and needs to be renamed and updated in the stubs (and possibly the various subclasses need to be added as well). More generically, there should probably be a test that checks that the stubs agree with the actual implementation, at least so long as the mypy type information is in a separate file from the implementation. I'll go ahead and put in a pull request this afternoon that hopefully fixes the specific issue with the type stubs for _BaseDataLoaderIter, this is my first pytorch issue so bear with me :) — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#26673?email_source=notifications&email_token=AAHRFPIF5EWAHI74MYRIPUDQMJKIVA5CNFSM4IZTBI22YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD764CUQ#issuecomment-536723794>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAHRFPPNDKCNL3NOFMORVP3QMJKIVANCNFSM4IZTBI2Q> .

[ngoldbaum]

I guess ultimately the issue here is that only one file -- aten/src/ATen/function_wrapper.py -- is getting type checked by the pytorch CI at the moment (see mypy-files.txt in the root of the repo). Fixing this issue in general therefore means making mypy run over the whole codebase, which is #16574, and seems like a much bigger job than the narrow issues with the _DataLoaderIter pointed out here. I think after #27105 is merged this can be closed as a dupe of #16574?

[ngoldbaum]

The specific issue pointed out here has been fixed, so I'm going to close this in favor of #16574 which tracks broader improvements to type correctness.