Stanford cars #5166
Stanford cars #5166
Conversation
💊 CI failures summary and remediationsAs of commit 121bb55 (more details on the Dr. CI page):
2 failures not recognized by patterns:
1 job timed out:
🚧 1 ongoing upstream failure: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 a lot @abhi-glitchhg for the PR. I did an initial round of review and will do another once my comments are addressed.
I'm using only one file (cars_train_annos.mat) out of all files in root/devkit. so do I have to reproduce fake data files for all files in devkit or just one fake cars_train_annos.mat is sufficient?
Unless the other files could interfere with your implementation you don't need to "mock" them. Since you select the specific file by name rather than say list all files and select one by a partial name, I think you can safely ignore them here.
|
Regarding the code format and the failing CI job: we have a couple of autoformatters that handle this for you. Have a look at our contribution guide. |
This comment was marked as resolved.
This comment was marked as resolved.
Yes, that is wrong. Are you working on Windows by any chance? Anyway, you can disable it with |
| class_names = meta_data["class_names"][0] | ||
| return {class_name[0].replace(" ", "_").replace("/", "_"): i for i, class_name in enumerate(class_names)} | ||
|
|
||
| def _make_dataset(self): |
There was a problem hiding this comment.
| def _make_dataset(self): | |
| def _make_dataset(self) -> List[Tuple[str, int]]: |
This comment was marked as resolved.
This comment was marked as resolved.
|
im using windows and conda virtual environment |
This comment was marked as resolved.
This comment was marked as resolved.
|
@abhi-glitchhg Whatever you did, CI is happy with the code format now. You don't have to do anything else in that regard. Anyway, that is on us to investigate and fix. Thanks a lot for reporting the issue. |
There was a problem hiding this comment.
Hey @abhi-glitchhg, besides the test failures there are only minor comments left from my side.
In addition, could you remove most of the comments you added in the code? We should only add comments if they add information to the code rather than to just describe what the code does. I'll give you an example below.
|
@abhi-glitchhg thank you so much for your work on this PR! I took the liberty to fix some minor failures, and do a few cleanups in 8fceb0b (changes look big but I mostly just moved stuff around, you had done all the hard work already!). Perhaps @pmeier can take a final look at it and I'll merge when green? |
There was a problem hiding this comment.
Two nits, but nothing blocking. Thanks a lot @abhi-glitchhg and @NicolasHug!
|
Hey @NicolasHug! You merged this PR, but no labels were added. The list of valid labels is available at https://github.com/pytorch/vision/blob/main/.github/process_commit.py |
Summary: * [WIP] *added stanford_cars * [WIP] added stanfordCars to docs * [WIP] minor edits * [WIP] minor edits * edited StanfordCars class * Adding Testcase for stanford cars * Added Testcase for stanford cars * Added Testcase for stanford cars * minor edit * made changes as per the suggestions * fixed typo in naming stanford_cars.py * cars_meta.mat file will be created in test * Some cleanups * Sigh * don't convert to strings Reviewed By: datumbox, NicolasHug Differential Revision: D33655259 fbshipit-source-id: 48882109cb3e21bccdba34eef9ef17e7468b78c4 Co-authored-by: Nicolas Hug <[email protected]>
#5108
I have added Stanford Cars dataset
cc @pmeier