Don't export outside annotations #1729
Conversation
|
@zhiltsov-max could you fix the tests? |
zhiltsov-max
changed the title
zhiltsov-max
changed the title
| last_shape = track.shapes[-1] | ||
| if last_shape.frame < int(task_data.meta['task']['stop_frame']): | ||
| track.shapes.append(last_shape._replace(outside=True, | ||
| frame=last_shape.frame + task_data.frame_step) |
There was a problem hiding this comment.
I'm not sure that here we need to add task_data.frame_step (probably +1 is correct). You can get a shape with frame > task_data.meta['task']['stop_frame']. @azhavoro , could you please look at the line and give your opinion?
There was a problem hiding this comment.
With +1 I'm not sure about how filtered out frames would be handled in UI, e.g. in "go to the next keyframe" button.
There was a problem hiding this comment.
@zhiltsov-max , could you please check?
There was a problem hiding this comment.
I've checked. We can't just save the next frame (which is +1) without breaking the logic in many places, at least on the server, so we probably should stick to +frame_step.
| @@ -410,9 +410,10 @@ def match_frame(self, filename): | |||
| "Cannot match filename or determine frame number for {} filename".format(filename)) | |||
|
|
|||
| class CvatTaskDataExtractor(datumaro.SourceExtractor): | |||
| def __init__(self, task_data, include_images=False): | |||
| def __init__(self, task_data, include_images=False, include_outside=False): | |||
There was a problem hiding this comment.
Why do we need the flag at all? When is it useful?
| dm_dataset = datumaro.components.project.Dataset.from_extractors(extractor) | ||
| self.assertEqual(4, len(dm_dataset.get("image_1").annotations)) | ||
|
|
||
| extractor = CvatTaskDataExtractor(task_data, include_outside=True) |
There was a problem hiding this comment.
I believe we always should include_outside but different formats can use or ignore such shapes.
There was a problem hiding this comment.
Maybe, but there is no formats with such annotation property.
There was a problem hiding this comment.
I believed it too, so we've been there already. It means that all formats must expect (read as "support") such shapes. Much better is to filter them on the caller's side.
There was a problem hiding this comment.
Now "caller" knows about format implementation details (a format doesn't support "outside" flag). Is it OK? What is about attributes? Tracks? Cuboids? Are you going to filter that as?
There was a problem hiding this comment.
I think, only CVAT knows about outside, so I treat it as a CVAT implementation detail. Tracks and cuboids are not exported too.
There was a problem hiding this comment.
OK. I see that CvatTaskDataExtractor is used as an utility function by each format. Thus the knowledge encapsulated.
Pull Request Test Coverage Report for Build 6330
💛 - Coveralls |
Motivation and context
Fixes #1620
outsideannotations were not considered during export.How has this been tested?
Unit tests.
Checklist
developbranchcvat-core, cvat-data and cvat-ui)
License
Feel free to contact the maintainers if that's a concern.