Replace references to _DataLoaderIter with _BaseDataLoaderIter #27105
Conversation
pytorchbot
added
module: dataloader
torch/utils/data/dataloader.pyi
Outdated
| # We quote '_DataLoaderIter' since it isn't defined yet and the definition can't be moved up since | ||
| # '_DataLoaderIter' references 'DataLoader'. Pending updates of PEP 484 will fix this. | ||
| def __iter__(self) -> '_DataLoaderIter':... | ||
| def __iter__(self) -> _BaseDataLoaderIter:... |
There was a problem hiding this comment.
Has the problem mentioned in previous comment been solved?
There was a problem hiding this comment.
I think so? I wasn’t able to reproduce the error in the comments with the latest version of mypy as well as a version from April, I was running:
mypy torch/utils/data/dataloader.py
and didn’t see any errors from mypy. That invocation may not have been sufficient to trigger the error @malmaud saw when they originally wrote this code.
There was a problem hiding this comment.
At some point relatively recently, Python/mypy added the ability to handle unquoted forward references, but there are definitely versions which cannot. I think it's safer to keep the quotes unless I was actually in error before and this isn't a forward reference.
There was a problem hiding this comment.
@ngoldbaum mind updating this? :)
There was a problem hiding this comment.
Done, I also expanded the comment to reflect the current situation in mypy so future readers are hopefully a little less confused.
sublcasses `_SingleProcessDataLoaderIter` and `_MultiprocessingDataLoaderIter`. However, the type stubs were not updated at the same time. This renames the stub for `_DataLoaderIter` with the new name for the base class.
There was a problem hiding this comment.
@ezyang is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
…ch#27105) Summary: Back in April, malmaud added type annotations for `dataloader.py`. However, at about the same time, SsnL in pytorch#19228 replaced `_DataLoaderIter` with `_BaseDataLoaderIter` and two subclasses, `_SingleProcessDataLoaderIter`, and `_MultiProcessingDataLoaderIter`. However - probably because these changes happened in parallel at roughly the same time, the type stubs and several other references in the codebase were never updated to match this refactoring. I've gone ahead and done the updates to reflect the refactoring in pytorch#19228, which fixes the specific type stub/impelementation mismatch pointed out in pytorch#26673, although not the broader problem that pytorch doesn't have a test to make sure that the `.pyi` type stub files match the real API defined in `.py` files. Pull Request resolved: pytorch#27105 Differential Revision: D17813641 Pulled By: ezyang fbshipit-source-id: ed7ac025c8d6ad3f298dd073347ec83bb4b6600c
Back in April, @malmaud added type annotations for
dataloader.py. However, at about the same time, @ssnl in #19228 replaced_DataLoaderIterwith_BaseDataLoaderIterand two subclasses,_SingleProcessDataLoaderIter, and_MultiProcessingDataLoaderIter. However - probably because these changes happened in parallel at roughly the same time, the type stubs and several other references in the codebase were never updated to match this refactoring.I've gone ahead and done the updates to reflect the refactoring in #19228, which fixes the specific type stub/impelementation mismatch pointed out in #26673, although not the broader problem that pytorch doesn't have a test to make sure that the
.pyitype stub files match the real API defined in.pyfiles.