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

Remove error when test dataloader used in test #1495

Merged
merged 32 commits into from
Apr 16, 2020
Merged

Remove error when test dataloader used in test #1495

merged 32 commits into from
Apr 16, 2020

Conversation

williamFalcon
Copy link
Contributor

@williamFalcon williamFalcon commented Apr 15, 2020

When we removed test dataloader we forgot to remove the error check

Fixes #1496

@mergify mergify bot requested a review from a team April 15, 2020 00:38
@pep8speaks
Copy link

pep8speaks commented Apr 15, 2020

Hello @williamFalcon! Thanks for updating this PR.

Line 1026:111: E501 line too long (116 > 110 characters)

Comment last updated at 2020-04-15 23:36:15 UTC

@williamFalcon williamFalcon changed the title remove error when test dataloader used in test [WIP] remove error when test dataloader used in test Apr 15, 2020
@Borda Borda added the feature Is an improvement or enhancement label Apr 15, 2020
@williamFalcon williamFalcon changed the title [WIP] remove error when test dataloader used in test Remove error when test dataloader used in test Apr 15, 2020
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.

mind to have the new test structure as separate PR...

@mergify mergify bot requested a review from a team April 15, 2020 15:59
@williamFalcon williamFalcon added bug Something isn't working priority: 0 High priority task labels Apr 15, 2020
@williamFalcon
Copy link
Contributor Author

williamFalcon commented Apr 15, 2020

@Borda Trail is not intuitive. Please leave as EvalModel

@Borda
Copy link
Member

Borda commented Apr 15, 2020

@Borda Trail is not intuitive. Please leave as EvaluationTemplateModel

ok, we just need to separate and we cannot use work test

  • test as a phase from a model live cycle
  • test as our case in testing

so can I have just TemplateModel because in this moment the evaluation is associated with the test as a model live cycle?

@@ -111,6 +111,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).

### Removed

- Removed test for no test dataloader in .fit ([#1495](https://github.com/PyTorchLightning/pytorch-lightning/pull/1495))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- Removed test for no test dataloader in .fit ([#1495](https://github.com/PyTorchLightning/pytorch-lightning/pull/1495))
- Removed test for no test dataloader in `.fit()` ([#1495](https://github.com/PyTorchLightning/pytorch-lightning/pull/1495))

@mergify mergify bot requested a review from a team April 16, 2020 00:03
@williamFalcon williamFalcon merged commit 3431c62 into master Apr 16, 2020
@Borda Borda deleted the warm branch April 16, 2020 07:21
tullie pushed a commit to tullie/pytorch-lightning that referenced this pull request Jun 7, 2020
* remove error when test dataloader used in test

* remove error when test dataloader used in test

* remove error when test dataloader used in test

* remove error when test dataloader used in test

* remove error when test dataloader used in test

* remove error when test dataloader used in test

* fix lost model reference

* remove error when test dataloader used in test

* fix lost model reference

* moved optimizer types

* moved optimizer types

* moved optimizer types

* moved optimizer types

* moved optimizer types

* moved optimizer types

* moved optimizer types

* moved optimizer types

* added tests for warning

* fix lost model reference

* fix lost model reference

* added tests for warning

* added tests for warning

* refactoring

* refactoring

* fix imports

* refactoring

* fix imports

* refactoring

* fix tests

* fix mnist

* flake8

* review

Co-authored-by: J. Borovec <jirka.borovec@seznam.cz>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working feature Is an improvement or enhancement priority: 0 High priority task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect MisconfigurationException for models without dataloaders.
3 participants