-
Notifications
You must be signed in to change notification settings - Fork 541
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Labelstudio instances support #2706
Labelstudio instances support #2706
Conversation
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@
## develop #2706 +/- ##
========================================
Coverage 61.67% 61.67%
========================================
Files 254 254
Lines 42851 42851
Branches 346 346
========================================
Hits 26429 26429
Misses 16422 16422
Flags with carried forward coverage won't be shown. Click here to find out more. Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution! Left some small comments for you 😄
@@ -575,11 +578,12 @@ def generate_labeling_config(media, label_type, labels=None): | |||
return config_str | |||
|
|||
|
|||
def import_label_studio_annotation(result): | |||
def import_label_studio_annotation(result, label_type): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you make label_type=None
the default, since this extra information is only needed when deciding between semantic and instance segmentations?
@@ -624,7 +628,7 @@ def _update_dict(src_dict, update_dict): | |||
return new | |||
|
|||
|
|||
def export_label_to_label_studio(label, full_result=None): | |||
def export_label_to_label_studio(sample, label_field, label_type, full_result=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
full_result
already includes original_width
and original_height
, so can you just use that and avoid changing the signature of this method?
) | ||
|
||
brush = fou.lazy_import( | ||
"label_studio_converter.brush", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: lazy_import()
can be placed at the very top of the module with other imports, since the module won't actually be imported unless it is used.
I notice that local lazy_import()
is used elsewhere in this module, perhaps you could clean that up too? 🤗
"label_studio_converter.brush", | ||
callback=lambda: fou.ensure_import("label_studio_converter.brush"), | ||
) | ||
rle = brush.mask2rle(label.to_segmentation(frame_size=size).get_mask()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please install the dev dependencies? Basically just run:
bash install.bash -d
This will install pre-commit hooks that will automatically run some code formatting tools when you commit your edits.
@jbadger3 thanks for the contribution! I'm going to go ahead and merge this and the maintainers will handle the remaining TODOs 🤗 |
Thanks Brian. Sorry I was so slow in getting the other TODOs done. Family (toddlers and infants) are cutting into my free time:) |
What changes are proposed in this pull request?
Resolves #2689 and adds support for the 'instances'
label_type
using Brush based annotations in Label Studio.How is this patch tested? If it is not, please explain why.
Tested using code given in #2689.
I was not able to run linting on my machine, so if one the maintainers could run linting for me it would be much appreciated!
Release Notes
Is this a user-facing change that should be mentioned in the release notes?
notes for FiftyOne users.
label_type
of 'instances'. Prior to this fix would lead to bounding box annotations. Now sets up brush based (mask) annotations as indicated in the docs.What areas of FiftyOne does this PR affect?
fiftyone
Python library changes