Check for duplicate Reference IDs on Frame and Scene creation (#295)

Author: jean-lucas

CHANGELOG.md

@@ -5,6 +5,12 @@ All notable changes to the [Nucleus Python Client](https://github.com/scaleapi/n
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [0.10.7](https://github.com/scaleapi/nucleus-python-client/releases/tag/v0.10.7) - 2022-05-09
+
+### Fixed
+- Add checks for duplicate reference IDs
+
+
 ## [0.10.6](https://github.com/scaleapi/nucleus-python-client/releases/tag/v0.10.6) - 2022-05-06
 
 ### Added

nucleus/dataset.py

@@ -554,6 +554,9 @@ def append(
         ]
         lidar_scenes = [item for item in items if isinstance(item, LidarScene)]
         video_scenes = [item for item in items if isinstance(item, VideoScene)]
+
+        check_for_duplicate_reference_ids(dataset_items)
+
         if dataset_items and (lidar_scenes or video_scenes):
             raise Exception(
                 "You must append either DatasetItems or Scenes to the dataset."
@@ -573,8 +576,6 @@ def append(
                 video_scenes, update, asynchronous
             )
 
-        check_for_duplicate_reference_ids(dataset_items)
-
         if len(dataset_items) > WARN_FOR_LARGE_UPLOAD and not asynchronous:
             print(
                 "Tip: for large uploads, get faster performance by importing your data "

nucleus/dataset_item.py

@@ -195,13 +195,17 @@ def check_all_paths_remote(dataset_items: Sequence[DatasetItem]):
 def check_for_duplicate_reference_ids(dataset_items: Sequence[DatasetItem]):
     ref_ids = []
     for dataset_item in dataset_items:
-        if dataset_item.reference_id is not None:
-            ref_ids.append(dataset_item.reference_id)
+        if dataset_item.reference_id is None:
+            raise ValueError(
+                f"Reference ID cannot be None. Encountered DatasetItem with no reference ID:\n{dataset_item}"
+            )
+        ref_ids.append(dataset_item.reference_id)
     if len(ref_ids) != len(set(ref_ids)):
         duplicates = {
             f"{key}": f"Count: {value}"
             for key, value in Counter(ref_ids).items()
+            if value > 1
         }
         raise ValueError(
-            f"Duplicate reference ids found among dataset_items: {duplicates}"
+            f"Duplicate reference IDs found among dataset_items: {duplicates}"
         )

nucleus/scene.py

@@ -18,13 +18,17 @@
 )
 
 from .annotation import is_local_path
-from .dataset_item import DatasetItem, DatasetItemType
+from .dataset_item import (
+    DatasetItem,
+    DatasetItemType,
+    check_for_duplicate_reference_ids,
+)
 
 
 class Frame:
-    """Collection of sensor data pertaining to a single timestep.
+    """Collection of sensor data pertaining to a single time step.
 
-    For 3D data, each Frame hosues a sensor-to-data mapping and must have exactly
+    For 3D data, each Frame houses a sensor-to-data mapping and must have exactly
     one pointcloud with any number of camera images.
 
     Parameters:
@@ -37,16 +41,15 @@ class Frame:
     """
 
     def __init__(self, **kwargs):
-        self.items = {}
+        self.items: Dict[str, DatasetItem] = {}
         for key, value in kwargs.items():
-            self.items[key] = value
-
-    def __post_init__(self):
-        for key, value in self.items.items():
             assert isinstance(key, str), "All keys must be names of sensors"
             assert isinstance(
                 value, DatasetItem
-            ), "All values must be DatasetItems"
+            ), f"All values must be DatasetItems, instead got type {type(value)}"
+            self.items[key] = value
+
+        check_for_duplicate_reference_ids(list(self.items.values()))
 
     def __repr__(self) -> str:
         return f"Frame(items={self.items})"
@@ -129,6 +132,8 @@ def __post_init__(self):
         if self.metadata is None:
             self.metadata = {}
 
+        self.validate()
+
     def __eq__(self, other):
         return all(
             [
@@ -151,10 +156,14 @@ def num_sensors(self) -> int:
     def validate(self):
         # TODO: make private
         assert self.length > 0, "Must have at least 1 frame in a scene"
+        all_items = []
         for frame in self.frames_dict.values():
             assert isinstance(
                 frame, Frame
             ), "Each frame in a scene must be a Frame object"
+            all_items.extend(frame.get_items())
+
+        check_for_duplicate_reference_ids(all_items)
 
     def add_item(
         self, index: int, sensor_name: str, item: DatasetItem

pyproject.toml

@@ -21,7 +21,7 @@ exclude = '''
 
 [tool.poetry]
 name = "scale-nucleus"
-version = "0.10.6"
+version = "0.10.7"
 description = "The official Python client library for Nucleus, the Data Platform for AI"
 license =  "MIT"
 authors = ["Scale AI Nucleus Team <nucleusapi@scaleapi.com>"]

tests/test_dataset.py

@@ -352,7 +352,7 @@ def test_raises_error_for_duplicate():
         )
     assert (
         str(error.value)
-        == "Duplicate reference ids found among dataset_items:"
+        == "Duplicate reference IDs found among dataset_items:"
         " {'duplicate': 'Count: 2'}"
     )
 

tests/test_scene.py

@@ -161,11 +161,10 @@ def test_scene_property_methods():
 
 def test_scene_add_item():
     scene_ref_id = "scene_1"
-    scene = LidarScene(scene_ref_id)
+    firstFrame = Frame(lidar=TEST_LIDAR_ITEMS[0])
+    scene = LidarScene(scene_ref_id, frames=[firstFrame])
     scene.add_item(0, "camera", TEST_DATASET_ITEMS[0])
-    scene.add_item(0, "lidar", TEST_LIDAR_ITEMS[0])
     scene.add_item(1, "lidar", TEST_LIDAR_ITEMS[1])
-
     assert set(scene.get_sensors()) == set(["camera", "lidar"])
     assert scene.get_item(1, "lidar") == TEST_LIDAR_ITEMS[1]
     assert scene.get_items_from_sensor("lidar") == [