-
Notifications
You must be signed in to change notification settings - Fork 297
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
Update Metrics/Write Preds Testing #697
Conversation
Hello @pruksmhc! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
You can repair most issues by installing black and running: Comment last updated at 2019-06-19 01:15:43 UTC |
… into check_update_metrics
Ready for another review @sleepinyourhat |
Last bit of cleanup, plus make sure tests pass. |
…update_metrics
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.
Some minor things
environment.yml
Outdated
@@ -16,6 +16,7 @@ dependencies: | |||
- tensorboard | |||
- tensorboardX==1.2 | |||
- sendgrid==5.4.1 | |||
- pyhocon==0.3.35 |
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.
Not sure why this is appearing in this diff - this is the same as master.
tests/test_write_preds.py
Outdated
self.wic.val_data = [ | ||
Instance( | ||
{ | ||
"sent1_str": MetadataField("Room and board yo."), |
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.
is this a real example??
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.
Oh nope :P , deleting the yo
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'm not super familiar with testing, so do makes sure to test these and address the minor comments.
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.
Some cleanup (in addition to Alex's requests), but happy to approve. Merge when ready.
…update_metrics
… into check_update_metrics
* adding initial tests * adding initial tests * black formatting * adding more than 1 val_preds, changing sts-b example * reduced to 2 examples for update_metrics * batch siz eof one test, cleaning up tests * changing directory for .CircleCI builds * removed extra changes * fixing WiC.jsonl name in write_preds_test * black * nit picking * black styling
Tests to determine that updating metrics and writing predictions works as expected.