Split annotation types into more specific ones#10543
Split annotation types into more specific ones#10543zhiltsov-max wants to merge 6 commits intodevelopfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
|
||
| class Meta: | ||
| abstract = True | ||
| default_permissions = () |
There was a problem hiding this comment.
Is this actually needed? I thought it was just a holdover from Django's permission system (which we don't use).
There was a problem hiding this comment.
I removed it. but I think it should be removed everywhere, if we're not using it.
| related_name='attributes', related_query_name='attribute') | ||
|
|
||
| class TrackedShape(Shape): | ||
| class TrackedShape(FrameAnnotationMixin, ShapeAnnotationMixin): |
There was a problem hiding this comment.
It's a little weird to have this include annotation mixins when it is not itself an annotation... Maybe "Annotation" should be removed from the base class names or replaced with something else?
There was a problem hiding this comment.
These mixins are supposed for annotation classes, and it's reflected in the class names. This class is an annotation, even though it's not inherited from Annotation. I think there could be another annotation base class, but it would only contain id or be empty, so it doesn't look like a good addition so far.
| class ScoredAnnotationSerializer(serializers.Serializer): | ||
| score = serializers.FloatField(min_value=0, max_value=1, default=1) | ||
|
|
||
| class ImageAnnotationSerializer(serializers.Serializer): |
There was a problem hiding this comment.
| class ImageAnnotationSerializer(serializers.Serializer): | |
| class FrameAnnotationSerializer(serializers.Serializer): |
This would be consistent with the model mixin.
| data['value'] = str(data['value']) | ||
| return super().to_internal_value(data) | ||
|
|
||
| class AttributedAnnotationSerializer(serializers.Serializer): |
There was a problem hiding this comment.
This is just a nitpick, but IMO these mini-serializers should be moved after AnnotationSerializer for consistency with models.py.
There was a problem hiding this comment.
I think it's a little bit overkill. The files are modified independently, I don't think it makes sense to maintain the order of type definitions without strong reasons for this.
2 participants
Motivation and context
Split annotation types into more detailed subclasses for easier combination.
How has this been tested?
Checklist
developbranchLicense
Feel free to contact the maintainers if that's a concern.