Refactor: static annotations #5
Conversation
Finished Yolo + LS converts on the basic annotation level + added tests for them
|
Noticed that there's no code for creating an LS task from IR annotations, fixing that. |
|
|
||
| filename: Optional[str] = None | ||
|
|
||
| category: str |
There was a problem hiding this comment.
I feel like this should be on the individual annotation types - for example, maybe a bounding box might have multiple non-conflicting classes. Or the user might want to store a more general mapping of {category:confidence} or similar to that.
- Do you benefit from having this in the base class?
- Did you provide a way to have optional custom freestyle information in the annotations? How hard would that be (including for the backend)?
There was a problem hiding this comment.
Do you benefit from having this in the base class?
Yes, because I need it if I want to e.g. gather all the classes in the annotations.
I can turn this into
@abstractmethod
def get_categories(self) -> list[str]:
...and it will work, but I want us to be aware of WHAT this will be used for.
If you want to store confidences like that, how do you expect to convert it to any other format?
Did you provide a way to have optional custom freestyle information in the annotations?
What for? If that optional information can't be used for conversion, then why have it?
I can add a meta: dict[str, Any] into any annotation, but then it's going to be a stringly-typed mess IF we want to utilize it at any point.
I understand your point that the classes here are non-extensible though.
How hard would that be (including for the backend)?
I didn't give a lot of thought for how this works on the backend, since this isn't interacting with anything for now. What do you mean by backend? I don't expect to be sending this IR to the backend/DE for storage.
There was a problem hiding this comment.
I mean, in the end this gets converted to LS format and sent to the backend. We shouldn't put anything in there which will prevent LS from reading the task for the common image labeling formats - I'm guessing it will just ignore any additional fields
| class IRPosePoint(BaseModel): | ||
| x: float | ||
| y: float | ||
| visible: Optional[bool] = None |
There was a problem hiding this comment.
What is this? Isn't it a display setting? Or does it mean something like "The user's hand is out of frame but I would estimate that it's in coordinate (-42,-314) outside the image bounds?"
There was a problem hiding this comment.
CVAT calls it occluded I think, YOLO calls it visible.
I don't think it particularily refers to out-of-bounds points, but instead to occlusion. If a point is not visible, that means something is in front of the hand and you can't see it on the picture.
There was a problem hiding this comment.
Makes sense. Occluded is more clear.
| from .skeleton import parse_skeleton | ||
| from dagshub_annotation_converter.ir.image import IRAnnotationBase | ||
|
|
||
| CVATParserFunction = Callable[[ElementBase, ElementBase], IRAnnotationBase] |
There was a problem hiding this comment.
You probably want to rename this module to cvat/xml2 to disambiguate
There was a problem hiding this comment.
I'm out of the loop here, can you let me know what's the thing it's supposed to disambiguate with? They have multiple formats?
There was a problem hiding this comment.
Yes they can output to multiple formats, and the one we support AFAIK is "CVAT for images" xml, I guess 1.1 not 2.0 actually
https://docs.cvat.ai/docs/manual/advanced/formats/
dagshub_annotation_converter/formats/label_studio/ir_conversion.py
Outdated
Show resolved
Hide resolved
Change name of the state argument to coordinate_style to match with the enum
| if label_extension is not None: | ||
| # Replace the extension | ||
| ext = get_extension(in_path) | ||
| if ext is None: | ||
| return None | ||
| new_parts[-1] = new_parts[-1].replace(ext, label_extension) |
There was a problem hiding this comment.
This isn't described in the function name or docstring - classic case of better no comment than a wrong one
There was a problem hiding this comment.
Also, it searches for the extension in the whole path instead of just its final part, which is a bug if there's any dot in a dir name
There was a problem hiding this comment.
get_extension only looks for the dot in the last part of the path (name = path.name at the beginning of that function)
# Conflicts: # dagshub_annotation_converter/__init__.py # dagshub_annotation_converter/image/exporters/yolo.py # dagshub_annotation_converter/image/importers/dagshub_datasource.py # dagshub_annotation_converter/image/util/path_util.py # dagshub_annotation_converter/schema/ir/annotation_ir.py # dagshub_annotation_converter/schema/label_studio/abc.py # dagshub_annotation_converter/schema/label_studio/keypointlabels.py # dagshub_annotation_converter/schema/label_studio/polygonlabels.py # dagshub_annotation_converter/schema/label_studio/rectanglelabels.py # dagshub_annotation_converter/schema/label_studio/task.py
Goal of this refactor: go away from a nested class structure to a flatter structure,
Now instead of having project -> files -> annotations, everything is a list of annotations, which contain enough stuff to define the annotation, and additional per-framework stuff is provided in a context (right now it's only needed in YOLO here).
Tried to provide as much testing as possible, so hope it's easy enough to read.
Not implemented in this: DagsHub datasource converters, I'm putting that in the client instead.