Add DTD dataset #5115
Add DTD dataset #5115
Conversation
💊 CI failures summary and remediationsAs of commit 7d21152 (more details on the Dr. CI page):
1 failure not recognized by patterns:
🚧 4 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. |
There was a problem hiding this comment.
Thanks @pmeier , this looks great. I made some minor comment, the only one I'm still not sure about is whether we should allow fold="all". LMK what you think
| if path.name == "labels_joint_anno.txt": | ||
| return 1 | ||
|
|
||
| return 0 | ||
| elif path.parents[1].name == "images": | ||
| return 2 |
There was a problem hiding this comment.
instead of hardcoded 0 1 2, would a small private enum be overkill here? or maybe just hardcoded named constants? No strong opinion but this might help readability
There was a problem hiding this comment.
Yes, that could work. If that is ok with you, I'll put it in my backlog, because we should do that for all datasets that use a Demultiplexer.
Co-authored-by: Nicolas Hug <[email protected]>
There was a problem hiding this comment.
Thanks @pmeier , LGTM with the potential renaming of fold to partition as we just discussed offline.
Summary: * add DTD as prototype dataset * add old style dataset * add test for old dataset * fix tests for windows * add dataset to docs * remove properties and use pathlib * Apply suggestions from code review * fold -> partition Reviewed By: sallysyw Differential Revision: D33479261 fbshipit-source-id: d879da77bd33064e57841a5f7ffdcea62c3add36 Co-authored-by: Nicolas Hug <[email protected]> Co-authored-by: Nicolas Hug <[email protected]>
Addresses #5108 and supersedes #743. I've also added the corresponding prototype dataset.
cc @pmeier @bjuncek