-
Notifications
You must be signed in to change notification settings - Fork 814
Add support for QQP dataset with unit tests #1713
Conversation
parmeet
left a comment
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.
LGTM!
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.
LGTM. Can we also make sure to add the sharding filter to the dataset as @parmeet mentioned here #1710 (comment).
Btw I think we can land irrespective of the test failures coming from linux (since this is caused by some recent changes related to torchdata). As long as all the tests are passing on mac os.
| file_name = "quora_duplicate_questions.tsv" | ||
| txt_file = os.path.join(base_dir, file_name) | ||
| mocked_data = [] | ||
| print(txt_file) |
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.
Can we remove the print statement here
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.
Solved with #1734
| def setUpClass(cls): | ||
| super().setUpClass() | ||
| cls.root_dir = cls.get_base_temp_dir() | ||
| print(cls.root_dir) |
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.
Remove print
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.
Solved with #1734
Summary
Test
pytest test/datasets/test_qqp.pyContext
See #1710