Skip to content

Fix CVAT text attributes coerced to int on annotation import #7252

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

Open
joaquinhuigomez wants to merge 4 commits into
base: develop
Choose a base branch
from
+53 −10
Open

Fix CVAT text attributes coerced to int on annotation import #7252

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6b21492
Fix text attributes coerced to int on CVAT annotation import
joaquinhuigomez Mar 28, 2026
73e5eb4
Fix text attribute coercion in CVAT annotation parsing
joaquinhuigomez Mar 28, 2026
9ff28ab
Treat empty text scalars as unset in _parse_value
joaquinhuigomez Mar 28, 2026
9f2259f
Add coerce_text_attrs parameter to control text attribute handling
joaquinhuigomez Apr 2, 2026
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
63 changes: 53 additions & 10 deletions fiftyone/utils/cvat.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3358,11 +3358,13 @@ def upload_annotations(self, samples, anno_key, launch_editor=False):

return results

def download_annotations(self, results):
def download_annotations(self, results, coerce_text_attrs=False):
api = self.connect_to_api()

logger.info("Downloading labels from CVAT...")
annotations = api.download_annotations(results)
annotations = api.download_annotations(
results, coerce_text_attrs=coerce_text_attrs
)
Comment on lines +3361 to +3367
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Apr 2, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Expose coerce_text_attrs on the public load path.

fiftyone.utils.annotations.load_annotations() still calls results.backend.download_annotations(results) with no way to pass this flag, and the abstract AnnotationBackend.download_annotations() signature still omits it. As written, the legacy opt-in is unreachable from the standard API even though the backend/API now expose it. Please thread the option through the public helper or persist it on the run config/results.

Also applies to: 4649-4657

🧰 Tools
🪛 Ruff (0.15.7)

[warning] 3361-3361: Boolean default positional argument in function definition

(FBT002)

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@fiftyone/utils/cvat.py` around lines 3361 - 3367, The public load path
doesn't expose the coerce_text_attrs flag: update
fiftyone.utils.annotations.load_annotations() to accept a coerce_text_attrs
parameter (default False) and thread it through when calling
results.backend.download_annotations(results,
coerce_text_attrs=coerce_text_attrs); also update the abstract method signature
AnnotationBackend.download_annotations(self, results, coerce_text_attrs=False)
and concrete implementations (e.g., CVATBackend.download_annotations /
fiftyone/utils/cvat.py::download_annotations) to accept and pass the flag;
alternatively persist the choice on the results/run config and read it inside
results.backend.download_annotations if you prefer configuration-based
propagation.

logger.info("Download complete")

return annotations
Expand Down Expand Up @@ -4644,12 +4646,15 @@ def upload_samples(self, samples, anno_key, backend):

return results

def download_annotations(self, results):
def download_annotations(self, results, coerce_text_attrs=False):
"""Download the annotations from the CVAT server for the given results
instance and parses them into the appropriate FiftyOne types.

Args:
results: a :class:`CVATAnnotationResults`
coerce_text_attrs (False): whether to coerce text attributes to
numeric types. By default, text attribute values are preserved
as strings

Returns:
the annotations dict
Expand Down Expand Up @@ -4719,10 +4724,13 @@ def download_annotations(self, results):
frame_stop -= offset

# Download task data
attr_id_map, _class_map_rev = self._get_attr_class_maps(
task_id
attr_id_map, attr_type_map, _class_map_rev = (
self._get_attr_class_maps(task_id)
)
Comment on lines +4727 to 4729
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 28, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Apply the same type-aware parsing to XML imports.

This fix only works for the REST download path, where attr_type is available from task metadata. Line 1702 and Line 2436 still call _parse_value() without type metadata, so load_cvat_image_annotations() and load_cvat_video_annotations() will still coerce digit-only text attrs from XML imports.

Also applies to: 7693-7695

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@fiftyone/utils/cvat.py` around lines 4722 - 4724, XML import paths call
_parse_value without type metadata causing digit-only text attributes to be
coerced; in load_cvat_image_annotations and load_cvat_video_annotations, fetch
the attribute maps via self._get_attr_class_maps(task_id) (to obtain
attr_type_map) and pass that attr_type_map into calls to _parse_value (instead
of calling _parse_value(value) with no type info). Update the places that
currently call _parse_value without type metadata to use _parse_value(value,
attr_type_map, attr_id_map) (or the existing _parse_value signature that accepts
type maps) so parsing is type-aware; also apply the same change to the other XML
import call sites that use _parse_value without attr_type_map (references:
functions _parse_value, load_cvat_image_annotations,
load_cvat_video_annotations, and _get_attr_class_maps).


if coerce_text_attrs:
attr_type_map = None

job_ids = self._get_job_ids(task_id)
for job_id in job_ids:
job_resp = self.get(self.job_annotation_url(job_id)).json()
Expand Down Expand Up @@ -4797,6 +4805,7 @@ def download_annotations(self, results):
frame_stop,
frame_step,
assigned_scalar_attrs=scalar_attrs,
attr_type_map=attr_type_map,
)
label_field_results = self._merge_results(
label_field_results, tag_results
Expand All @@ -4818,6 +4827,7 @@ def download_annotations(self, results):
assigned_scalar_attrs=scalar_attrs,
occluded_attrs=_occluded_attrs,
group_id_attrs=_group_id_attrs,
attr_type_map=attr_type_map,
)
label_field_results = self._merge_results(
label_field_results, shape_results
Expand Down Expand Up @@ -4851,6 +4861,7 @@ def download_annotations(self, results):
immutable_attrs=immutable_attrs,
occluded_attrs=_occluded_attrs,
group_id_attrs=_group_id_attrs,
attr_type_map=attr_type_map,
)
label_field_results = self._merge_results(
label_field_results, track_shape_results
Expand Down Expand Up @@ -4891,16 +4902,21 @@ def _get_attr_class_maps(self, task_id):
labels = self._get_task_labels(task_id)
_class_map = {}
attr_id_map = {}
attr_type_map = {}
for label in labels:
_class_map[label["id"]] = label["name"]
attr_id_map[label["id"]] = {
i["name"]: i["id"] for i in label["attributes"]
}
attr_type_map[label["id"]] = {
i["id"]: i.get("input_type", None)
for i in label["attributes"]
}

# AL: not sure why we didn't just reverse keys/vals initially
class_map_rev = {n: i for i, n in _class_map.items()}

return attr_id_map, class_map_rev
return attr_id_map, attr_type_map, class_map_rev

def _get_paginated_results(self, base_url, get_page_url=None, value=None):
results = []
Expand Down Expand Up @@ -5808,6 +5824,7 @@ def _parse_shapes_tags(
immutable_attrs=None,
occluded_attrs=None,
group_id_attrs=None,
attr_type_map=None,
):
results = {}
prev_type = None
Expand Down Expand Up @@ -5848,6 +5865,7 @@ def _parse_shapes_tags(
immutable_attrs=immutable_attrs,
occluded_attrs=occluded_attrs,
group_id_attrs=group_id_attrs,
attr_type_map=attr_type_map,
)

# For non-outside tracked objects, the last track goes to the end of
Expand Down Expand Up @@ -5883,6 +5901,7 @@ def _parse_shapes_tags(
immutable_attrs=immutable_attrs,
occluded_attrs=occluded_attrs,
group_id_attrs=group_id_attrs,
attr_type_map=attr_type_map,
)

return results
Expand All @@ -5907,6 +5926,7 @@ def _parse_annotation(
immutable_attrs=None,
occluded_attrs=None,
group_id_attrs=None,
attr_type_map=None,
):
frame = anno["frame"]

Expand Down Expand Up @@ -5949,6 +5969,7 @@ def _parse_annotation(
occluded_attrs=occluded_attrs,
group_id_attrs=group_id_attrs,
group_id=track_group_id,
attr_type_map=attr_type_map,
)

# Non-keyframe annotations were interpolated from keyframes but
Expand Down Expand Up @@ -6030,11 +6051,20 @@ def _parse_annotation(
if expected_label_type == "scalar":
label_type = "scalar"
if assigned_scalar_attrs:
_attr_types = (
attr_type_map.get(anno["label_id"], {})
if attr_type_map
else {}
)
num_attrs = len(anno["attributes"])
attr_ind = 0
while label is None and attr_ind < num_attrs:
attr = anno["attributes"][attr_ind]
attr_type = _attr_types.get(
attr["spec_id"], None
)
label = _parse_value(
anno["attributes"][attr_ind]["value"]
attr["value"], attr_type=attr_type
)
attr_ind += 1
if label is not None:
Expand All @@ -6054,7 +6084,10 @@ def _parse_annotation(
label = class_map[anno["label_id"]]
else:
label_type = "classifications"
cvat_tag = CVATTag(anno, class_map, attr_id_map, server_id_map)
cvat_tag = CVATTag(
anno, class_map, attr_id_map, server_id_map,
attr_type_map=attr_type_map,
)
label = cvat_tag.to_classification()

if label is None or label_type in ignore_types:
Expand Down Expand Up @@ -7074,6 +7107,7 @@ def __init__(
attr_id_map,
server_id_map,
attributes=None,
attr_type_map=None,
):
cvat_id = label_dict["label_id"]
server_id = label_dict["id"]
Expand All @@ -7093,9 +7127,13 @@ def __init__(

# Parse attributes
attr_id_map_rev = {v: k for k, v in attr_id_map[cvat_id].items()}
_attr_types = (
attr_type_map.get(cvat_id, {}) if attr_type_map else {}
)
for attr in attrs:
name = attr_id_map_rev[attr["spec_id"]]
value = _parse_value(attr["value"])
attr_type = _attr_types.get(attr["spec_id"], None)
value = _parse_value(attr["value"], attr_type=attr_type)
if value is not None:
if name.startswith("attribute:"):
name = name[len("attribute:") :]
Expand Down Expand Up @@ -7177,13 +7215,15 @@ def __init__(
occluded_attrs=None,
group_id_attrs=None,
group_id=None,
attr_type_map=None,
):
super().__init__(
label_dict,
class_map,
attr_id_map,
server_id_map,
attributes=immutable_attrs,
attr_type_map=attr_type_map,
)

self.frame_size = ()
Expand Down Expand Up @@ -7658,7 +7698,10 @@ def _from_int_bool(value):
return None


def _parse_value(value):
def _parse_value(value, attr_type=None):
if attr_type == "text":
return None if value == "" else str(value)

try:
return int(value)
except:
Expand Down