Skip to content
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.

Sign up for GitHub

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

Fixing import and export issues with invalid tracks #8553

Merged
merged 14 commits into from
Oct 24, 2024
Merged

Fixing import and export issues with invalid tracks #8553

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
d944c8d
fixing invalid tracks
Eldies Oct 17, 2024
85ca560
adding test_annotations to format_python_code.sh
Eldies Oct 17, 2024
cbe1cac
changelog entry
Eldies Oct 17, 2024
35674dd
compare shapes only when necessary
Eldies Oct 17, 2024
3ae9a00
fixing sonarcloud issue
Eldies Oct 17, 2024
2f8f72a
small change in a test
Eldies Oct 17, 2024
a94f5eb
Merge branch 'refs/heads/develop' into dl/fix-invalid-tracks
Eldies Oct 17, 2024
2ea5e79
Merge branch 'refs/heads/develop' into dl/fix-invalid-tracks
Eldies Oct 21, 2024
8d76235
Merge branch 'refs/heads/develop' into dl/fix-invalid-tracks
Eldies Oct 21, 2024
0c27398
combining ifs
Eldies Oct 22, 2024
13c559a
Merge branch 'refs/heads/develop' into dl/fix-invalid-tracks
Eldies Oct 23, 2024
0d429d0
Merge branch 'refs/heads/develop' into dl/fix-invalid-tracks
Eldies Oct 23, 2024
1e35536
changed changelog entry
Eldies Oct 23, 2024
2ec6756
remark fix
Eldies Oct 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions changelog.d/20241017_134221_dmitrii.lavrukhin_fix_invalid_tracks.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
### Fixed

- Track shape duplication on import
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now there should be no errors during export tracks with duplicated shapes. Why not mention that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rewritten it a bit

(<https://github.com/cvat-ai/cvat/pull/8553>)
6 changes: 5 additions & 1 deletion cvat/apps/dataset_manager/annotation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,8 @@ def filter_track_shapes(shapes):
if last_key >= stop and scoped_shapes[-1]['points'] != segment_shapes[-1]['points']:
segment_shapes.append(scoped_shapes[-1])
elif scoped_shapes[-1]['keyframe'] and \
scoped_shapes[-1]['outside']:
scoped_shapes[-1]['outside'] and \
(len(segment_shapes) == 0 or scoped_shapes[-1]['frame'] > segment_shapes[-1]['frame']):
segment_shapes.append(scoped_shapes[-1])
elif stop + 1 < len(interpolated_shapes) and \
interpolated_shapes[stop + 1]['outside']:
Expand Down Expand Up @@ -950,6 +951,9 @@ def propagate(shape, end_frame, *, included_frames=None):
break # The track finishes here

if prev_shape:
if curr_frame == prev_shape["frame"]:
if dict(shape, id=None, keyframe=None) == dict(prev_shape, id=None, keyframe=None):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These 2 if statements can be combined into one.
Why do we need to exclude keyframe when comparing shapes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They can be combined, but then this line becomes too long (137 cahracters) and should be split to 2 lines anyway, and I think it is more readable the way it is now.
keyframe should be excluded, because it is always True in prev_shape due to shape["keyframe"] = True on line 967.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They can be combined, but then this line becomes too long (137 cahracters) and should be split to 2 lines anyway, and I think it is more readable the way it is now.

if (
    condition1
    and condition2
):
    ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, combined them

continue
assert curr_frame > prev_shape["frame"], f"{curr_frame} > {prev_shape['frame']}. Track id: {track['id']}" # Catch invalid tracks

# Propagate attributes
Expand Down
168 changes: 129 additions & 39 deletions cvat/apps/dataset_manager/tests/test_annotation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@
#
# SPDX-License-Identifier: MIT

from cvat.apps.dataset_manager.annotation import TrackManager

from unittest import TestCase

from cvat.apps.dataset_manager.annotation import AnnotationIR, TrackManager
from cvat.apps.engine.models import DimensionType


class TrackManagerTest(TestCase):
def _check_interpolation(self, track):
interpolated = TrackManager.get_interpolated_shapes(track, 0, 7, '2d')
interpolated = TrackManager.get_interpolated_shapes(track, 0, 7, "2d")

self.assertEqual(
[
Expand All @@ -24,7 +25,7 @@ def _check_interpolation(self, track):
[
{k: v for k, v in shape.items() if k in ["frame", "keyframe", "outside"]}
for shape in interpolated
]
],
)

def test_point_interpolation(self):
Expand All @@ -41,25 +42,25 @@ def test_point_interpolation(self):
"type": "points",
"occluded": False,
"outside": False,
"attributes": []
"attributes": [],
},
{
"frame": 2,
"attributes": [],
"points": [3.0, 4.0, 5.0, 6.0],
"type": "points",
"occluded": False,
"outside": True
"outside": True,
},
{
"frame": 4,
"attributes": [],
"points": [3.0, 4.0, 5.0, 6.0],
"type": "points",
"occluded": False,
"outside": False
"outside": False,
},
]
],
}

self._check_interpolation(track)
Expand All @@ -78,25 +79,25 @@ def test_polygon_interpolation(self):
"type": "polygon",
"occluded": False,
"outside": False,
"attributes": []
"attributes": [],
},
{
"frame": 2,
"attributes": [],
"points": [3.0, 4.0, 5.0, 6.0, 7.0, 6.0, 4.0, 5.0],
"type": "polygon",
"occluded": False,
"outside": True
"outside": True,
},
{
"frame": 4,
"attributes": [],
"points": [3.0, 4.0, 5.0, 6.0, 7.0, 6.0, 4.0, 5.0],
"type": "polygon",
"occluded": False,
"outside": False
"outside": False,
},
]
],
}

self._check_interpolation(track)
Expand All @@ -116,7 +117,7 @@ def test_bbox_interpolation(self):
"type": "rectangle",
"occluded": False,
"outside": False,
"attributes": []
"attributes": [],
},
{
"frame": 2,
Expand All @@ -125,7 +126,7 @@ def test_bbox_interpolation(self):
"rotation": 0,
"type": "rectangle",
"occluded": False,
"outside": True
"outside": True,
},
{
"frame": 4,
Expand All @@ -134,9 +135,9 @@ def test_bbox_interpolation(self):
"rotation": 0,
"type": "rectangle",
"occluded": False,
"outside": False
"outside": False,
},
]
],
}

self._check_interpolation(track)
Expand All @@ -156,7 +157,7 @@ def test_line_interpolation(self):
"type": "polyline",
"occluded": False,
"outside": False,
"attributes": []
"attributes": [],
},
{
"frame": 2,
Expand All @@ -165,7 +166,7 @@ def test_line_interpolation(self):
"rotation": 0,
"type": "polyline",
"occluded": False,
"outside": True
"outside": True,
},
{
"frame": 4,
Expand All @@ -174,9 +175,9 @@ def test_line_interpolation(self):
"rotation": 0,
"type": "polyline",
"occluded": False,
"outside": False
"outside": False,
},
]
],
}

self._check_interpolation(track)
Expand All @@ -196,7 +197,7 @@ def test_outside_bbox_interpolation(self):
"type": "rectangle",
"occluded": False,
"outside": False,
"attributes": []
"attributes": [],
},
{
"frame": 2,
Expand All @@ -214,9 +215,9 @@ def test_outside_bbox_interpolation(self):
"type": "rectangle",
"occluded": False,
"outside": True,
"attributes": []
}
]
"attributes": [],
},
],
}

expected_shapes = [
Expand All @@ -228,7 +229,7 @@ def test_outside_bbox_interpolation(self):
"occluded": False,
"outside": False,
"attributes": [],
"keyframe": True
"keyframe": True,
},
{
"frame": 1,
Expand All @@ -238,7 +239,7 @@ def test_outside_bbox_interpolation(self):
"occluded": False,
"outside": False,
"attributes": [],
"keyframe": False
"keyframe": False,
},
{
"frame": 2,
Expand All @@ -248,7 +249,7 @@ def test_outside_bbox_interpolation(self):
"occluded": False,
"outside": True,
"attributes": [],
"keyframe": True
"keyframe": True,
},
{
"frame": 4,
Expand All @@ -258,11 +259,11 @@ def test_outside_bbox_interpolation(self):
"occluded": False,
"outside": True,
"attributes": [],
"keyframe": True
}
"keyframe": True,
},
]

interpolated_shapes = TrackManager.get_interpolated_shapes(track, 0, 5, '2d')
interpolated_shapes = TrackManager.get_interpolated_shapes(track, 0, 5, "2d")
self.assertEqual(expected_shapes, interpolated_shapes)

def test_outside_polygon_interpolation(self):
Expand All @@ -279,17 +280,17 @@ def test_outside_polygon_interpolation(self):
"type": "polygon",
"occluded": False,
"outside": False,
"attributes": []
"attributes": [],
},
{
"frame": 2,
"points": [3.0, 4.0, 5.0, 6.0, 7.0, 8.0],
"type": "polygon",
"occluded": False,
"outside": True,
"attributes": []
}
]
"attributes": [],
},
],
}

expected_shapes = [
Expand All @@ -300,7 +301,7 @@ def test_outside_polygon_interpolation(self):
"occluded": False,
"outside": False,
"attributes": [],
"keyframe": True
"keyframe": True,
},
{
"frame": 1,
Expand All @@ -309,7 +310,7 @@ def test_outside_polygon_interpolation(self):
"occluded": False,
"outside": False,
"attributes": [],
"keyframe": False
"keyframe": False,
},
{
"frame": 2,
Expand All @@ -318,9 +319,98 @@ def test_outside_polygon_interpolation(self):
"occluded": False,
"outside": True,
"attributes": [],
"keyframe": True
}
"keyframe": True,
},
]

interpolated_shapes = TrackManager.get_interpolated_shapes(track, 0, 3, "2d")
self.assertEqual(expected_shapes, interpolated_shapes)

def test_duplicated_shape_interpolation(self):
# there should not be any new tracks with duplicated shapes,
# but it is possible that the database still contains some
expected_shapes = [
{
"type": "rectangle",
"occluded": False,
"outside": False,
"points": [100, 100, 200, 200],
"frame": 0,
"attributes": [],
"rotation": 0,
},
{
"type": "rectangle",
"occluded": False,
"outside": True,
"points": [100, 100, 200, 200],
"frame": 1,
"attributes": [],
"rotation": 0,
},
]
track = {
"id": 666,
"frame": 0,
"group": None,
"source": "manual",
"attributes": [],
"elements": [],
"label": "cat",
"shapes": expected_shapes + [expected_shapes[-1]],
}

interpolated_shapes = TrackManager.get_interpolated_shapes(track, 0, 3, '2d')
interpolated_shapes = TrackManager.get_interpolated_shapes(track, 0, 2, "2d")
self.assertEqual(expected_shapes, interpolated_shapes)


class AnnotationIRTest(TestCase):
def test_slice_track_does_not_duplicate_outside_frame_on_the_end(self):
track_shapes = [
{
"type": "rectangle",
"occluded": False,
"outside": False,
"points": [100, 100, 200, 200],
"frame": 0,
"attributes": [],
"rotation": 0,
},
{
"type": "rectangle",
"occluded": False,
"outside": True,
"points": [100, 100, 200, 200],
"frame": 1,
"attributes": [],
"rotation": 0,
},
{
"type": "rectangle",
"occluded": False,
"outside": False,
"points": [111, 111, 222, 222],
"frame": 10,
"attributes": [],
"rotation": 0,
},
]
data = {
"tags": [],
"shapes": [],
"tracks": [
{
"id": 666,
"frame": 0,
"group": None,
"source": "manual",
"attributes": [],
"elements": [],
"label": "cat",
"shapes": track_shapes,
}
],
}
annotation = AnnotationIR(dimension=DimensionType.DIM_2D, data=data)
sliced_annotation = annotation.slice(0, 1)
self.assertEqual(sliced_annotation.data["tracks"][0]["shapes"], track_shapes[0:2])
1 change: 1 addition & 0 deletions dev/format_python_code.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ for paths in \
"cvat/apps/engine/default_settings.py" \
"cvat/apps/engine/field_validation.py" \
"cvat/apps/engine/model_utils.py" \
"cvat/apps/dataset_manager/tests/test_annotation.py" \
; do
${BLACK} -- ${paths}
${ISORT} -- ${paths}
Expand Down
Loading