-
Notifications
You must be signed in to change notification settings - Fork 68
video classification keyframe #345
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
Conversation
…into ms/video-classification-keyframe
@@ -24,13 +24,25 @@ class ClassificationAnswer(FeatureSchema): | |||
- Represents a classification option. | |||
- Because it inherits from FeatureSchema | |||
the option can be represented with either the name or feature_schema_id | |||
|
|||
- The key frame arg only applies to video classifications. | |||
Each answer can have a key frame indepdent of the others. |
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.
indepdent -> independent
'value': answer.value | ||
}) for answer in self.answer | ||
]) | ||
return Dropdown(answer=[answer.to_common() for answer in self.answer]) |
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.
probably not related to this pr... but Dropdown only support 1 answer similar to Radio
answers rather than Checklist
is there a reason why we have a list of answers for Dropdown?
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 think it is a list because that is how we export it
elif not self._is_url(): | ||
return TextData(text=self.row_data, **data_row_info) | ||
return 'image' | ||
elif (self._row_contains((".txt", ".text", ".html")) and |
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 this be an or statement with line 198? if self._has_text_annotations():
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.
We would also need to check if there are not object annotations. This also would make the condition on 200 harder to read.
else: | ||
# no annotations to infer data type from. | ||
# Use information from the row_data format if possible. | ||
if self._row_contains((".jpg", ".png", ".jpeg")) and self._is_url(): |
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 this be an or statement with line 200? elif self._has_object_annotations():
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.
We could but I think this is more readable
Uh oh!
There was an error while loading. Please reload this page.