Added KITTI dataset #3640
Added KITTI dataset #3640
Conversation
|
I see tests failing due to pandas import error. Should I add it to config.yml? |
There was a problem hiding this comment.
Thanks @prabhat00155 , I just made a quick review.
Regarding the dependency on pandas: once the test is ported to use ImageDatasetTestCase, you can set REQUIRED_PACKAGES = ("pandas",). Feel free to take a look at the CelebATestCase class which also relies on pd.read_csv.
There was a problem hiding this comment.
The PR looks great, thanks a lot @prabhat00155 !
I've made a few more comments, let me know what you think
…the returned values.
|
Windows test failures seem related, can you have a look? |
There was a problem hiding this comment.
Thanks @prabhat00155 ,
The tests look great!
I think you'll also need to add the dataset in docs/source/datasets.rst so that the docs are rendered properly. You can check the docs locally or in this PR: click on the build_docs CI run, and then "Artifacts" tab
Pinging @pmeier for another look ;)
There was a problem hiding this comment.
Looks great to me, thanks a lot @prabhat00155 !
Would be good to have @pmeier have a look as well
prabhat00155
force-pushed
the
prabhat00155/add_kitti_dataset
branch
from
de72936 to
2920913
Compare
There was a problem hiding this comment.
@prabhat00155 Looks good overall! I have a few comments below, though.
@pmeier Addressed your comments, could you take a look? Thanks! |
There was a problem hiding this comment.
One minor thing left. Otherwise LGTM. Thanks a lot @prabhat00155!
|
Thanks a ton @prabhat00155 ! |
Summary: * Added KITTI dataset * Addressed review comments * Changed type of target to List[Dict] and corrected the data types of the returned values. * Updated unit test to rely on ImageDatasetTestCase * Added kitti to dataset documentation * Cleaned up test and some minor changes * Made data_url a string instead of a list * Removed unnecessary try and print Reviewed By: NicolasHug Differential Revision: D27706941 fbshipit-source-id: aa646f17e7ad5a0858320274cc2ec226fa8f4790 Co-authored-by: Francisco Massa <fvsmassa@gmail.com>
Discussion: #3562