Skip to content
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

generalize reinstantiation of dataloader #1346

Merged
merged 5 commits into from
Apr 3, 2020

Conversation

justusschock
Copy link
Member

@justusschock justusschock commented Apr 2, 2020

Before submitting

  • Was this discussed/approved via a Github issue? (no need for typos and docs improvements)
  • Did you read the contributor guideline, Pull Request section?
  • Did you make sure to update the docs?
  • Did you write any new necessary tests?
  • If you made a notable change (that affects users), did you update the CHANGELOG?

What does this PR do?

Generalizes reinstantiation of data loaders to allow subclasses of data loader

PR review

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

Did you have fun?

Make sure you had fun coding 🙃

@justusschock justusschock self-assigned this Apr 2, 2020
@mergify mergify bot requested a review from a team April 2, 2020 15:09
@mergify
Copy link
Contributor

mergify bot commented Apr 2, 2020

This pull request is now in conflict... :(

Copy link
Member

@Borda Borda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀

CHANGELOG.md Outdated Show resolved Hide resolved
@Borda Borda added the feature Is an improvement or enhancement label Apr 2, 2020
@Borda Borda added this to the 0.7.2 milestone Apr 2, 2020
@Borda Borda added the ready PRs ready to be merged label Apr 2, 2020
@Borda Borda requested a review from a team April 3, 2020 07:27
@mergify
Copy link
Contributor

mergify bot commented Apr 3, 2020

This pull request is now in conflict... :(

@williamFalcon
Copy link
Contributor

@justusschock this is awesome. mind rebasing so we can get those GPU fixes from master?

@codecov
Copy link

codecov bot commented Apr 3, 2020

Codecov Report

Merging #1346 into master will increase coverage by <1%.
The diff coverage is 0%.

@@           Coverage Diff           @@
##           master   #1346    +/-   ##
=======================================
+ Coverage      92%     92%   +<1%     
=======================================
  Files          63      63            
  Lines        3316    3315     -1     
=======================================
  Hits         3040    3040            
+ Misses        276     275     -1

@Borda Borda force-pushed the dataloader_reinstantiation branch from 21ff726 to 79baaae Compare April 3, 2020 20:01
@Borda
Copy link
Member

Borda commented Apr 3, 2020

@williamFalcon resolved, @justusschock could you pls check it...

@justusschock
Copy link
Member Author

LGTM, thanks @Borda

@Borda
Copy link
Member

Borda commented Apr 3, 2020

@justusschock could you please approve it... @williamFalcon ^^

@williamFalcon williamFalcon merged commit f6a86e8 into master Apr 3, 2020
@Borda Borda deleted the dataloader_reinstantiation branch April 3, 2020 22:47
alexeykarnachev pushed a commit to alexeykarnachev/pytorch-lightning that referenced this pull request Apr 4, 2020
* generalize reinstantiation of dataloader

* fix condition

* add test

* update changelog

* fix changelog

Co-authored-by: J. Borovec <jirka.borovec@seznam.cz>
tullie pushed a commit to tullie/pytorch-lightning that referenced this pull request Jun 7, 2020
* generalize reinstantiation of dataloader

* fix condition

* add test

* update changelog

* fix changelog

Co-authored-by: J. Borovec <jirka.borovec@seznam.cz>
@Borda Borda removed this from the v0.7. milestone Apr 18, 2021
@Borda Borda added this to the v0.7.x milestone Apr 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Is an improvement or enhancement ready PRs ready to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants