Skip to content

Labelbox weedcoco support#326

Open
geezacoleman wants to merge 9 commits intomasterfrom
labelbox-coco
Open

Labelbox weedcoco support#326
geezacoleman wants to merge 9 commits intomasterfrom
labelbox-coco

Conversation

@geezacoleman
Copy link
Copy Markdown
Collaborator

Finished the README and labelbox -> VOC -> weedcoco converter.

Copy link
Copy Markdown
Contributor

@jnothman jnothman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A squiz at overall structure.

Comment thread weedcoco/validation.py
]
validate_json.ref_store = {obj["$id"]: obj for obj in schema_objects}
ref_store = validate_json.ref_store
schema_uri = MAIN_SCHEMAS[schema]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an incorrect merge resolution

Comment thread weedcoco/validation.py
# TODO


def validate(weedcoco, images_root=None, schema="weedcoco"):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an incorrect merge resolution

Comment thread weedcoco/importers/voc.py
import cv2
import os

visualisation = True
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should make this a command-line arg on all importers if we wish it to be most useful.

Comment thread Examples/voc_convert.bat
@@ -0,0 +1,30 @@
@echo off
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure we want this file in the repo

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeh wasn't sure about that, I didn't realise they could be potentially nasty files. Will remove.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's nasty, but it's a bit too specific for most users, while something like the README should be reasonably accessible???

ap.add_argument("--labelbox-json", required=True, type=str, help='path to downloaded labelbox JSON file')
ap.add_argument("--save-dir", required=True, type=str)
ap.add_argument("--keep-original-ID", action="store_true", default=False)
ap.add_argument("--voc-to-coco", action="store_true", default=False)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this optional?

def main():
ap = argparse.ArgumentParser(description=__doc__)

ap.add_argument("--labelbox-json", required=True, type=str, help='path to downloaded labelbox JSON file')
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you provide us a small example to test/confirm?


ap.add_argument("--labelbox-json", required=True, type=str, help='path to downloaded labelbox JSON file')
ap.add_argument("--save-dir", required=True, type=str)
ap.add_argument("--keep-original-ID", action="store_true", default=False)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

YAGNI?

ap.add_argument("--save-dir", required=True, type=str)
ap.add_argument("--keep-original-ID", action="store_true", default=False)
ap.add_argument("--voc-to-coco", action="store_true", default=False)
ap.add_argument("--start-index", default=0, type=int)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe call this resume-index?

ap.add_argument("--metadata-path", type=Path)
ap.add_argument("--agcontext-path", type=Path)
ap.add_argument("--save-image-dir", type=str, help='existing save directory for images')
ap.add_argument("--save-voc-dir", type=str, help='existing save directory for VOC files')
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not convinced we need the functionality of saving the VOC. Should be saved to a temp directory, then converted, in the present context of weedcoco?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It had been a bit of a hack workaround because both were available to me and it was simple to join one to the other, but yes just going straight to weedCOCO would be best maybe with the option to save VOC along the way? Or is that YAGNI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants