Skip to content

fix mypy typing errors in pytorch_lightning.utilities.data.py #13901

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Sep 14, 2022

Conversation

nandwalritik
Copy link
Contributor

What does this PR do?

  • mypy errors
src/pytorch_lightning/utilities/data.py:67: error: Incompatible types in "yield" (actual type "None", expected type "int")  [misc]
src/pytorch_lightning/utilities/data.py:109: error: Argument 1 to "len" has incompatible type "Union[DataLoader[Any], Iterable[Any]]"; expected "Sized"  [arg-type]
src/pytorch_lightning/utilities/data.py:119: error: Argument 1 to "has_iterable_dataset" has incompatible type "Union[DataLoader[Any], Iterable[Any]]"; expected "DataLoader[Any]"  [arg-type]
src/pytorch_lightning/utilities/data.py:131: error: Name "pl.Strategy" is not defined  [name-defined]
src/pytorch_lightning/utilities/data.py:138: error: Item "LightningDataModule" of "Union[LightningModule, LightningDataModule]" has no attribute "device"  [union-attr]
src/pytorch_lightning/utilities/data.py:190: error: Argument 2 to "_get_dataloader_init_args_and_kwargs" has incompatible type "Union[Sampler[Any], Iterable[Any]]"; expected "Optional[Sampler[Any]]"  [arg-type]
src/pytorch_lightning/utilities/data.py:241: error: Cannot access "__init__" directly  [misc]
src/pytorch_lightning/utilities/data.py:283: error: Incompatible types in assignment (expression has type "List[str]", variable has type "Set[str]")  [assignment]
src/pytorch_lightning/utilities/data.py:298: error: Incompatible types in assignment (expression has type "List[Any]", variable has type "Set[Any]")  [assignment]
src/pytorch_lightning/utilities/data.py:417: error: Argument 1 to "FastForwardSampler" has incompatible type "Optional[Sampler[Any]]"; expected "Union[Sampler[Any], Generator[Any, Any, Any]]"  [arg-type]
src/pytorch_lightning/utilities/data.py:534: error: Argument 1 to "get_len" has incompatible type "Dataset[Any]"; expected "DataLoader[Any]"  [arg-type]
Found 12 errors in 1 file (checked 240 source files)

Fix mypy errors attributed to pytorch_lightning.utilities.data.py for issue #13445

Does your PR introduce any breaking changes? If yes, please list them.

Before submitting

  • Was this discussed/approved via a GitHub issue? (not for typos and docs)
  • Did you read the contributor guideline, Pull Request section?
  • Did you make sure your PR does only one thing, instead of bundling different changes together?
  • Did you make sure to update the documentation with your changes? (if necessary)
  • Did you write any new necessary tests? (not for typos and docs)
  • Did you verify new and existing tests pass locally with your changes?
  • Did you list all the breaking changes introduced by this pull request?
  • Did you update the CHANGELOG? (not for typos, docs, test updates, or minor internal changes/refactors)

PR review

Anyone in the community is welcome to review the PR.
Before you start reviewing, make sure you have read the review guidelines. In short, see the following bullet-list:

  • Is this pull request ready for review? (if not, please submit in draft mode)
  • Check that all items from Before submitting are resolved
  • Make sure the title is self-explanatory and the description concisely explains the PR
  • Add labels and milestones (and optionally projects) to the PR so it can be classified

Did you have fun?

Yeah

@github-actions github-actions bot added the pl Generic label for PyTorch Lightning package label Jul 28, 2022
@otaj otaj mentioned this pull request Jul 29, 2022
52 tasks
@otaj otaj added this to the pl:1.7.x milestone Aug 2, 2022
@carmocca carmocca modified the milestones: pl:1.7.x, pl:1.8 Aug 3, 2022
@carmocca carmocca added the community This PR is from the community label Aug 3, 2022
@Borda
Copy link
Member

Borda commented Sep 12, 2022

@nandwalritik @otaj, mind checking the conflicts 🦦

@otaj
Copy link
Contributor

otaj commented Sep 13, 2022

Hi, @nandwalritik, do you intend to keep working on this? If not, I can take it over the finish line ⚡

@Borda
Copy link
Member

Borda commented Sep 13, 2022

Hi, @nandwalritik, do you intend to keep working on this? If not, I can take it over the finish line zap

let's take it over, pls 🐰

@mergify mergify bot removed the has conflicts label Sep 13, 2022
@nandwalritik
Copy link
Contributor Author

Hi, @nandwalritik, do you intend to keep working on this? If not, I can take it over the finish line ⚡

Sorry got busy with office work, you can take it over @otaj .

@otaj
Copy link
Contributor

otaj commented Sep 14, 2022

@Borda It should be finished now, ready for review.
@awaelchli I took the liberty of also typing lightning_lite/utilities/data.py since it's very much intertwined.

@awaelchli
Copy link
Contributor

@otaj Thank you, and sorry for the merge conflicts when ripping apart some of these utilities. I hope it wasn't too annoying.

@codecov
Copy link

codecov bot commented Sep 14, 2022

Codecov Report

Merging #13901 (2e0f6db) into master (71dadb5) will increase coverage by 2%.
The diff coverage is 84%.

@@            Coverage Diff            @@
##           master   #13901     +/-   ##
=========================================
+ Coverage      85%      86%     +2%     
=========================================
  Files         381      264    -117     
  Lines       27633    20035   -7598     
=========================================
- Hits        23372    17299   -6073     
+ Misses       4261     2736   -1525     

@carmocca carmocca enabled auto-merge (squash) September 14, 2022 10:52
@mergify mergify bot added the ready PRs ready to be merged label Sep 14, 2022
@carmocca carmocca merged commit 8e9780b into Lightning-AI:master Sep 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community This PR is from the community pl Generic label for PyTorch Lightning package ready PRs ready to be merged
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants