Use public API for loading in prototype datasets tests #5212
Conversation
💊 CI failures summary and remediationsAs of commit cbf128b (more details on the Dr. CI page): ✅ None of the CI failures appear to be your fault 💚
🚧 2 ongoing upstream failures:These were probably caused by upstream breakages that are not fixed yet.
This comment was automatically generated by Dr. CI (expand for details).Please report bugs/suggestions to the (internal) Dr. CI Users group. |
| def _parse_mock_data(self, mock_data_fn): | ||
| def wrapper(info, root, config): | ||
| mock_infos = mock_data_fn(info, root, config) | ||
| def _parse_mock_data(self, config, mock_infos): |
There was a problem hiding this comment.
This is just GitHub not picking up on the identation change. Before this was a decorator, but I changed it into a regular method. The actual body has not changed.
| num_samples = 5 if config.split == "train" else 3 | ||
|
|
||
| path = root / f"{config.split}.txt" | ||
| path = root / f"{config.split}.csv" |
There was a problem hiding this comment.
This change and everything below in this file are actual bugs in our mock data generation that were hidden by our custom loading logic.
There was a problem hiding this comment.
Do you know why the tests were passing before despite the files had the wrong name?
There was a problem hiding this comment.
Before the resources where not collected by the regular logic, but rather "hand-fed" to _make_datapipe. So as long as the data in mock_resources[0] corresponded to Dataset.resources(...)[0] the test suite didn't notice.
| num_samples = 5 if config.split == "train" else 3 | ||
|
|
||
| path = root / f"{config.split}.txt" | ||
| path = root / f"{config.split}.csv" |
There was a problem hiding this comment.
Do you know why the tests were passing before despite the files had the wrong name?
Summary: * refactor prototype dataset tests to use public API for loading * add explanation * use loop alternative Reviewed By: jdsgomes, prabhat00155 Differential Revision: D33739387 fbshipit-source-id: 1ea4f7e925d6c686fa937d6cab162076ed887bc3
| try: | ||
| self.info.check_dependencies() | ||
| except ModuleNotFoundError as error: | ||
| pytest.skip(str(error)) |
There was a problem hiding this comment.
This should have not been removed. The tests should be runnable even if no or not all third party dependencies are installed.
3 participants
Follow-up to #5207 (comment)
cc @pmeier @bjuncek