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.

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

Merge release/v1.0.1 to develop #4894

Merged
merged 14 commits into from
Oct 5, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
41 changes: 19 additions & 22 deletions docs/source/integrations/coco.rst
Original file line number Diff line number Diff line change
Expand Up @@ -192,18 +192,14 @@ file containing COCO-formatted labels to work with:

dataset = foz.load_zoo_dataset("quickstart")

# Classes list
classes = dataset.distinct("ground_truth.detections.label")

# The directory in which the dataset's images are stored
IMAGES_DIR = os.path.dirname(dataset.first().filepath)

# Export some labels in COCO format
dataset.take(5).export(
dataset.take(5, seed=51).export(
dataset_type=fo.types.COCODetectionDataset,
label_field="ground_truth",
labels_path="/tmp/coco.json",
classes=classes,
)

Now we have a ``/tmp/coco.json`` file on disk containing COCO labels
Expand All @@ -220,7 +216,7 @@ corresponding to the images in ``IMAGES_DIR``:
"licenses": [],
"categories": [
{
"id": 0,
"id": 1,
"name": "airplane",
"supercategory": null
},
Expand All @@ -229,9 +225,9 @@ corresponding to the images in ``IMAGES_DIR``:
"images": [
{
"id": 1,
"file_name": "001631.jpg",
"height": 612,
"width": 612,
"file_name": "003486.jpg",
"height": 427,
"width": 640,
"license": null,
"coco_url": null
},
Expand All @@ -241,14 +237,14 @@ corresponding to the images in ``IMAGES_DIR``:
{
"id": 1,
"image_id": 1,
"category_id": 9,
"category_id": 1,
"bbox": [
92.14,
220.04,
519.86,
61.89000000000001
34.34,
147.46,
492.69,
192.36
],
"area": 32174.135400000006,
"area": 94773.8484,
"iscrowd": 0
},
...
Expand All @@ -271,8 +267,9 @@ dataset:
include_id=True,
)

# Verify that the class list for our dataset was imported
print(coco_dataset.default_classes) # ['airplane', 'apple', ...]
# COCO categories are also imported
print(coco_dataset.info["categories"])
# [{'id': 1, 'name': 'airplane', 'supercategory': None}, ...]

print(coco_dataset)

Expand Down Expand Up @@ -319,16 +316,16 @@ to add them to your dataset as follows:
#
# Mock COCO predictions, where:
# - `image_id` corresponds to the `coco_id` field of `coco_dataset`
# - `category_id` corresponds to classes in `coco_dataset.default_classes`
# - `category_id` corresponds to `coco_dataset.info["categories"]`
#
predictions = [
{"image_id": 1, "category_id": 18, "bbox": [258, 41, 348, 243], "score": 0.87},
{"image_id": 2, "category_id": 11, "bbox": [61, 22, 504, 609], "score": 0.95},
{"image_id": 1, "category_id": 2, "bbox": [258, 41, 348, 243], "score": 0.87},
{"image_id": 2, "category_id": 4, "bbox": [61, 22, 504, 609], "score": 0.95},
]
categories = coco_dataset.info["categories"]

# Add COCO predictions to `predictions` field of dataset
classes = coco_dataset.default_classes
fouc.add_coco_labels(coco_dataset, "predictions", predictions, classes)
fouc.add_coco_labels(coco_dataset, "predictions", predictions, categories)

# Verify that predictions were added to two images
print(coco_dataset.count("predictions")) # 2
Expand Down
5 changes: 2 additions & 3 deletions docs/source/user_guide/dataset_creation/datasets.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1499,9 +1499,8 @@ where `labels.json` is a JSON file in the following format:
...
],
"categories": [
...
{
"id": 2,
"id": 1,
"name": "cat",
"supercategory": "animal",
"keypoints": ["nose", "head", ...],
Expand All @@ -1524,7 +1523,7 @@ where `labels.json` is a JSON file in the following format:
{
"id": 1,
"image_id": 1,
"category_id": 2,
"category_id": 1,
"bbox": [260, 177, 231, 199],
"segmentation": [...],
"keypoints": [224, 226, 2, ...],
Expand Down
5 changes: 2 additions & 3 deletions docs/source/user_guide/export_datasets.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1646,9 +1646,8 @@ where `labels.json` is a JSON file in the following format:
},
"licenses": [],
"categories": [
...
{
"id": 2,
"id": 1,
"name": "cat",
"supercategory": "animal"
},
Expand All @@ -1669,7 +1668,7 @@ where `labels.json` is a JSON file in the following format:
{
"id": 1,
"image_id": 1,
"category_id": 2,
"category_id": 1,
"bbox": [260, 177, 231, 199],
"segmentation": [...],
"score": 0.95,
Expand Down
62 changes: 34 additions & 28 deletions fiftyone/utils/coco.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def add_coco_labels(
sample_collection,
label_field,
labels_or_path,
classes,
categories,
label_type="detections",
coco_id_field=None,
include_annotation_id=False,
Expand All @@ -68,7 +68,7 @@ def add_coco_labels(
{
"id": 1,
"image_id": 1,
"category_id": 2,
"category_id": 1,
"bbox": [260, 177, 231, 199],

# optional
Expand All @@ -88,7 +88,7 @@ def add_coco_labels(
{
"id": 1,
"image_id": 1,
"category_id": 2,
"category_id": 1,
"bbox": [260, 177, 231, 199],
"segmentation": [...],

Expand All @@ -109,7 +109,7 @@ def add_coco_labels(
{
"id": 1,
"image_id": 1,
"category_id": 2,
"category_id": 1,
"keypoints": [224, 226, 2, ...],
"num_keypoints": 10,

Expand All @@ -129,8 +129,14 @@ def add_coco_labels(
will be created if necessary
labels_or_path: a list of COCO annotations or the path to a JSON file
containing such data on disk
classes: the list of class label strings or a dict mapping class IDs to
class labels
categories: can be any of the following:

- a list of category dicts in the format of
:meth:`parse_coco_categories` specifying the classes and their
category IDs
- a dict mapping class IDs to class labels
- a list of class labels whose 1-based ordering is assumed to
correspond to the category IDs in the provided COCO labels
label_type ("detections"): the type of labels to load. Supported values
are ``("detections", "segmentations", "keypoints")``
coco_id_field (None): this parameter determines how to map the
Expand Down Expand Up @@ -195,10 +201,14 @@ class labels
view.compute_metadata()
widths, heights = view.values(["metadata.width", "metadata.height"])

if isinstance(classes, dict):
classes_map = classes
if isinstance(categories, dict):
classes_map = categories
elif not categories:
classes_map = {}
elif isinstance(categories[0], dict):
classes_map = {c["id"]: c["name"] for c in categories}
else:
classes_map = {i: label for i, label in enumerate(classes)}
classes_map = {i: label for i, label in enumerate(categories, 1)}
Comment on lines +204 to +211
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Rename classes_map to categories_map for consistency

Since the parameter has been renamed from classes to categories, it's recommended to rename the variable classes_map to categories_map to maintain consistency and avoid confusion.

Apply this diff to rename the variable:

         if isinstance(categories, dict):
-            classes_map = categories
+            categories_map = categories
         elif not categories:
-            classes_map = {}
+            categories_map = {}
         elif isinstance(categories[0], dict):
-            classes_map = {c["id"]: c["name"] for c in categories}
+            categories_map = {c["id"]: c["name"] for c in categories}
         else:
-            classes_map = {i: label for i, label in enumerate(categories, 1)}
+            categories_map = {i: label for i, label in enumerate(categories, 1)}

Remember to update subsequent usages of classes_map to categories_map.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if isinstance(categories, dict):
classes_map = categories
elif not categories:
classes_map = {}
elif isinstance(categories[0], dict):
classes_map = {c["id"]: c["name"] for c in categories}
else:
classes_map = {i: label for i, label in enumerate(classes)}
classes_map = {i: label for i, label in enumerate(categories, 1)}
if isinstance(categories, dict):
categories_map = categories
elif not categories:
categories_map = {}
elif isinstance(categories[0], dict):
categories_map = {c["id"]: c["name"] for c in categories}
else:
categories_map = {i: label for i, label in enumerate(categories, 1)}


labels = []
for _coco_objects, width, height in zip(coco_objects, widths, heights):
Expand Down Expand Up @@ -563,15 +573,11 @@ def setup(self):
self.labels_path, extra_attrs=self.extra_attrs
)

classes = None
if classes_map is not None:
classes = _to_classes(classes_map)

if classes is not None:
info["classes"] = classes
info["classes"] = _to_classes(classes_map)

Comment on lines +577 to 578
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Update info["classes"] to info["categories"]

In line 578, the code assigns the class mappings to info["classes"]. To reflect the renaming from classes to categories, this should be updated to info["categories"].

Apply this diff to fix the inconsistency:

             if classes_map is not None:
-                info["classes"] = _to_classes(classes_map)
+                info["categories"] = _to_classes(classes_map)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
info["classes"] = _to_classes(classes_map)
info["categories"] = _to_classes(classes_map)

image_ids = _get_matching_image_ids(
classes,
classes_map,
images,
annotations,
image_ids=self.image_ids,
Expand Down Expand Up @@ -907,12 +913,11 @@ def export_sample(self, image_or_path, label, metadata=None):

def close(self, *args):
if self._dynamic_classes:
classes = sorted(self._classes)
labels_map_rev = _to_labels_map_rev(classes)
labels_map_rev = _to_labels_map_rev(sorted(self._classes))
for anno in self._annotations:
anno["category_id"] = labels_map_rev[anno["category_id"]]
else:
classes = self.classes
elif self.categories is None:
labels_map_rev = _to_labels_map_rev(self.classes)

Comment on lines +916 to 921
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

⚠️ Potential issue

Multiple occurrences of self.classes found across the codebase

The self.classes attribute is still present in several files, which may lead to potential AttributeError exceptions if not properly updated. Please address the following locations to ensure consistency and prevent errors:

  • fiftyone/zoo/datasets/base.py
  • fiftyone/utils/openimages.py
  • fiftyone/utils/yolo.py
  • fiftyone/utils/coco.py
  • fiftyone/utils/transformers.py
  • fiftyone/utils/tf.py
  • fiftyone/utils/kinetics.py
  • fiftyone/utils/torch.py
  • fiftyone/utils/eval/base.py
  • fiftyone/utils/clip/zoo.py
  • fiftyone/utils/data/parsers.py
  • fiftyone/utils/data/exporters.py
  • fiftyone/utils/data/importers.py
  • fiftyone/utils/eval/coco.py
  • fiftyone/utils/eval/classification.py
  • fiftyone/utils/eval/activitynet.py
  • fiftyone/utils/activitynet.py
  • fiftyone/core/collections.py

Please ensure all instances of self.classes are replaced with self.categories where appropriate.

🔗 Analysis chain

Replace self.classes with self.categories

At line 921, self.classes is used, but it should be self.categories due to the renaming. This prevents potential AttributeError exceptions.

Apply this diff to correct the variable:

             elif self.categories is None:
-                labels_map_rev = _to_labels_map_rev(self.classes)
+                labels_map_rev = _to_labels_map_rev(self.categories)

Run the following script to verify any remaining occurrences of self.classes:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Find all occurrences of 'self.classes' in the codebase.

# Expected result: No occurrences of 'self.classes' should remain.

rg --type python -F 'self.classes'

Length of output: 64


Script:

#!/bin/bash
# Description: Find all occurrences of 'self.classes' in Python files within the codebase.

rg --type py -F 'self.classes'

Length of output: 8709

_info = self.info or {}
_date_created = datetime.now().replace(microsecond=0).isoformat()
Expand All @@ -933,10 +938,10 @@ def close(self, *args):
categories = [
{
"id": i,
"name": l,
"name": c,
"supercategory": None,
}
for i, l in enumerate(classes)
for c, i in sorted(labels_map_rev.items(), key=lambda t: t[1])
]

labels = {
Expand Down Expand Up @@ -1681,7 +1686,7 @@ def download_coco_dataset_split(
if classes is not None:
# Filter by specified classes
all_ids, any_ids = _get_images_with_classes(
image_ids, annotations, classes, all_classes
image_ids, annotations, classes, all_classes_map
)
else:
all_ids = image_ids
Expand Down Expand Up @@ -1846,7 +1851,7 @@ def _parse_include_license(include_license):


def _get_matching_image_ids(
all_classes,
classes_map,
images,
annotations,
image_ids=None,
Expand All @@ -1862,7 +1867,7 @@ def _get_matching_image_ids(

if classes is not None:
all_ids, any_ids = _get_images_with_classes(
image_ids, annotations, classes, all_classes
image_ids, annotations, classes, classes_map
)
else:
all_ids = image_ids
Comment on lines +1870 to 1873
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Rename parameter classes to categories in _get_images_with_classes definition

The parameter classes in the _get_images_with_classes function should be renamed to categories to reflect the updated terminology.

Apply this diff to rename the parameter:

 def _get_images_with_classes(
         image_ids, annotations, target_classes, classes_map
-    ):
+    ):
+        # Rename target_classes to target_categories
+        target_categories = target_classes

And update the variable usage within the function accordingly.

Committable suggestion was skipped due to low confidence.

Expand Down Expand Up @@ -1930,7 +1935,7 @@ def _do_download(args):


def _get_images_with_classes(
image_ids, annotations, target_classes, all_classes
image_ids, annotations, target_classes, classes_map
):
if annotations is None:
logger.warning("Dataset is unlabeled; ignoring classes requirement")
Expand All @@ -1939,11 +1944,12 @@ def _get_images_with_classes(
if etau.is_str(target_classes):
target_classes = [target_classes]

bad_classes = [c for c in target_classes if c not in all_classes]
labels_map_rev = {c: i for i, c in classes_map.items()}

bad_classes = [c for c in target_classes if c not in labels_map_rev]
if bad_classes:
raise ValueError("Unsupported classes: %s" % bad_classes)

labels_map_rev = _to_labels_map_rev(all_classes)
class_ids = {labels_map_rev[c] for c in target_classes}

all_ids = []
Expand Down Expand Up @@ -2029,7 +2035,7 @@ def _load_image_ids_json(json_path):


def _to_labels_map_rev(classes):
return {c: i for i, c in enumerate(classes)}
return {c: i for i, c in enumerate(classes, 1)}


def _to_classes(classes_map):
Expand Down
Loading
Loading