-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Fix test configuration check and testing #1804
Conversation
Hello @rohitgr7! Thanks for updating this PR.
Comment last updated at 2020-05-13 13:11:07 UTC |
This pull request is now in conflict... :( |
Working on failed test cases. |
raise MisconfigurationException('You have passed in a `val_dataloader()`' | ||
' but have not defined `validation_step()`.') | ||
if not self.testing: | ||
if not self.is_overridden('training_step', model): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would consider moving this to __check_model_configuration_test
and alternatively bellow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest removing check_testing_model_configuration
because it's just a repeated code and testing configuration can be handled within check_model_configuration
. Also during testing check_model_configuration
is called which is unnecessary and was not working in #1720. The PR fixes both the issues.
Codecov Report
@@ Coverage Diff @@
## master #1804 +/- ##
======================================
- Coverage 88% 88% -0%
======================================
Files 71 71
Lines 4431 4422 -9
======================================
- Hits 3893 3885 -8
+ Misses 538 537 -1 |
Thank you! |
Before submitting
What does this PR do?
Fixes #1720 and #1754.
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 🙃