coco wrapper revised #176
coco wrapper revised #176
Conversation
| @@ -85,6 +85,10 @@ def run_tf_fp32(model_path, batch_size, num_runs, timeout, images_path, anno_pat | |||
|
|
|||
| def main(): | |||
| args = parse_args() | |||
|
|
|||
| if "COCO_IMG_PATH" or "COCO_ANNO_PATH" not in os.environ: | |||
There was a problem hiding this comment.
I would move this logic to download_coco function so it doesn't have to be written each time.
There was a problem hiding this comment.
Also as images and annotations must correspond I would assume that if any of the two has been set manually the other one must have been forgotten and the intention of user should not be ignored and overridden.
|
|
||
|
|
||
| def download_coco_dataset(): | ||
| from utils.downloads.utils import get_downloads_path |
There was a problem hiding this comment.
I would consider moving this function to COCO file and make it part of COCO dataset class
There was a problem hiding this comment.
It would be better to do this in anoter PR along with all of the other function of this type
utils/misc.py
Outdated
| dataset = pathlib.Path(coco_data, 'val2014') | ||
| labels = pathlib.Path(coco_data, 'annotations', 'instances_val2014.json') | ||
|
|
||
| if "COCO_IMG_PATH" not in os.environ: |
There was a problem hiding this comment.
If COCO_IMG_PATH in os.environ execution flow shouldn't arrive here at all
utils/misc.py
Outdated
| subprocess.run(["rm", 'val2014.zip']) | ||
| subprocess.run(["rm", 'annotations_trainval2014.zip']) | ||
| else: | ||
| pass |
There was a problem hiding this comment.
what's the else clause for?
utils/misc.py
Outdated
| subprocess.run(["wget", images_link]) | ||
| subprocess.run(["mkdir", coco_data]) | ||
| subprocess.run(["unzip", 'annotations_trainval2014.zip', '-d', coco_data]) | ||
| subprocess.run(["unzip", 'val2014.zip', '-d', coco_data]) |
There was a problem hiding this comment.
would be nice to additionally remove zip files after unpacking them
utils/misc.py
Outdated
|
|
||
| else: | ||
| if "COCO_IMG_PATH" not in os.environ: | ||
| print("COCO_IMG_PATH variable must be set along with COCO_ANNO_PATH!") |
There was a problem hiding this comment.
this is already handled elsewhere (in COCO utils file), not needed
There was a problem hiding this comment.
fair enough, I though your previous comment regarding this matter meant to remind the user that he set the one but not the other, but in this case this whole else statement will be removed and handled solely by coco.py
utils/misc.py
Outdated
|
|
||
| def download_coco_dataset(): | ||
| from utils.downloads.utils import get_downloads_path | ||
| if "COCO_IMG_PATH" not in os.environ and "COCO_ANNO_PATH" not in os.environ: |
There was a problem hiding this comment.
This is not a good test because user has the alternative ability to supply COCO paths through args instead of env variables. The execution of this function should be decided by COCODataset having ultimate knowledge of whether user has supplied paths on his own or not.
There was a problem hiding this comment.
This will also save the effort of putting this function's call into each and every COCO related run.py script.
No description provided.