pytorch-lightning icon indicating copy to clipboard operation
pytorch-lightning copied to clipboard

fixes typing errors in auto_restart.py

Open donlapark opened this issue 3 years ago • 1 comments

What does this PR do?

Fixes typing errors in pytorch_lightning/utilities/auto_restart.py as a part of #13445.

Well, this was quite a tough one, as the type of state_dict is not consistent throughout the file. Sometimes its type is Dict[str, Any], the other times, it is Dict[int, Any].

Also, the state attribute in MergedIteratorState can have two types: Dict[int, IteratorState] or Dict[str, Dict[int, IteratorState]], depending on whether the dataset is map-based or iterable. I have applied a band-aid fix which initially sets the type of state to Dict and then conditionally sets it to a more specific type in update method later.

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

Before submitting

  • [x] Was this discussed/approved via a GitHub issue? (not for typos and docs)
  • [x] Did you read the contributor guideline, Pull Request section?
  • [x] 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)
  • [x] Did you verify new and existing tests pass locally with your changes?
  • [x] 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?

Make sure you had fun coding 🙃

donlapark avatar Jul 28 '22 11:07 donlapark

Codecov Report

Merging #13904 (e5844dd) into master (66ca093) will increase coverage by 14%. The diff coverage is 95%.

:exclamation: Current head e5844dd differs from pull request most recent head e81d961. Consider uploading reports for the commit e81d961 to get more accurate results

@@            Coverage Diff            @@
##           master   #13904     +/-   ##
=========================================
+ Coverage      61%      76%    +14%     
=========================================
  Files         331      331             
  Lines       26621    26709     +88     
=========================================
+ Hits        16343    20253   +3910     
+ Misses      10278     6456   -3822     

codecov[bot] avatar Aug 06 '22 16:08 codecov[bot]

@carmocca mind review as code-owner?

rohitgr7 avatar Sep 01 '22 18:09 rohitgr7