-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
AA functions: add more stringent label/shape type compatibility checking #9005
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #9005 +/- ##
========================================
Coverage 73.40% 73.41%
========================================
Files 418 418
Lines 44296 44315 +19
Branches 3870 3870
========================================
+ Hits 32516 32532 +16
- Misses 11780 11783 +3
|
if not self._are_label_types_compatible(fun_output_type, ds_label.type): | ||
raise BadFunctionError( | ||
f"label {fun_label.name!r} has type {fun_output_type!r} in the function," | ||
f" but {ds_label.type!r} in the dataset" |
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.
Are you sure about the word "dataset"? It looks like it should be "destination task" instead. Probably, functions are supposed to annotate tasks or jobs.
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.
I use "dataset" everywhere in this module as a stand-in for "the thing we're annotating". Currently this is always a task, but I expect that per-job annotation will be implemented at some point, so I'm pre-emptively using a more generic word.
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.
Ok, but in either case, I think it can be confusing, as there is no any "dataset" so far. CVAT uses this word only for annotations in some format and in SDK dataset API.
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.
Yeah, I can see your point, but I don't think this is the PR to fix this in. Also... I don't know how to fix it anyway. I don't want to hardcode "task" everywhere, since _AnnotationMapper
is generic, and I'd like it to remain so.
and in SDK dataset API.
FWIW, there is a connection here, since we are using the dataset API behind the scenes (and _AnnotationMapper
is initialized with dataset.labels
).
Please resolve the conflicts. |
b4b1ea7
to
d391b87
Compare
|
Motivation and context
These changes are meant to enforce several conditions:
A function should not be able to create shapes of type incompatible with the type of the label it declares (for example, if a
function's label has type
rectangle
, it should not be able to create ellipses with that label).A function should not be able to create shapes of type incompatible with the type of the label in the task being annotated.
If a function's declared label has a type incompatible with the type of the corresponding task label, then it should not run at all (since it would be impossible for it to output a shape with that label that wouldn't violate either condition 1 or 2).
Altogether, these restrictions ensure that we don't create any shapes in a task that aren't compatible with that task's label types.
In addition, set explicit label types for the predefined functions.
How has this been tested?
Unit tests.
Checklist
develop
branch[ ] I have linked related issues (see GitHub docs)License
Feel free to contact the maintainers if that's a concern.