Skip to content

Fix: Add transformers for videos and bounding boxes#720

Open
ad-claw000 wants to merge 36 commits into
developfrom
fix/issue-335
Open

Fix: Add transformers for videos and bounding boxes#720
ad-claw000 wants to merge 36 commits into
developfrom
fix/issue-335

Conversation

@ad-claw000
Copy link
Copy Markdown
Contributor

Summary

Adds VideoProperties and BoundingBoxProperties transformers to handle AddVideo, AddBoundingBox, and AddPolygon commands.
Updates CommonProperties to process these new annotation types.

Verification

Manually verified that the property setting works properly for bounding boxes and polygons in the transformer workflow.

Fixes #335

Copilot AI review requested due to automatic review settings May 20, 2026 11:23
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds transformer support for non-image ingestion artifacts (videos and shape annotations) so common/static properties and derived metadata can be applied consistently during transformer-based ingestion pipelines (Fixes #335).

Changes:

  • Added VideoProperties transformer to compute basic blob-derived video metadata and ensure _Video.adb_data_source is indexed.
  • Added BoundingBoxProperties transformer to attach annotation-related properties to AddBoundingBox and AddPolygon commands.
  • Extended Transformer/CommonProperties to detect and apply properties to AddVideo, AddBoundingBox, and AddPolygon, and fixed ImageProperties property initialization/blob mapping.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
aperturedb/transformers/video_properties.py New transformer computing and attaching video metadata properties.
aperturedb/transformers/bounding_box_properties.py New transformer attaching annotation properties to bounding boxes and polygons.
aperturedb/transformers/transformer.py Tracks indices for AddVideo, AddBoundingBox, and AddPolygon commands.
aperturedb/transformers/image_properties.py Fixes property dict initialization and corrects blob indexing logic.
aperturedb/transformers/common_properties.py Applies common/static properties to images, videos, bboxes, and polygons.
aperturedb/transformers/init.py Introduces package exports for transformers (but currently has critical import issues).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread aperturedb/transformers/__init__.py Outdated
Comment thread aperturedb/transformers/__init__.py Outdated
Comment thread aperturedb/transformers/bounding_box_properties.py Outdated
Comment thread aperturedb/transformers/common_properties.py Outdated
Comment thread aperturedb/transformers/image_properties.py Outdated
Comment thread aperturedb/transformers/video_properties.py Outdated
Copilot AI review requested due to automatic review settings May 21, 2026 01:53
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

aperturedb/transformers/bounding_box_properties.py:35

  • This transformer iterates over _add_bounding_box_index / _add_polygon_index, which are computed from the first transaction only. If later transactions contain a different number/order of AddBoundingBox/AddPolygon commands (common for per-image variable annotation counts), some annotations won’t get annotation_source/annotation_mode applied, and transactions with fewer annotations can raise IndexError/KeyError (then skip the rest of the updates). Prefer iterating over the current transaction’s command list (x[0]) and applying properties to every command whose key is AddBoundingBox or AddPolygon.
            for ic in getattr(self, "_add_bounding_box_index", []):
                src_properties = x[0][ic]["AddBoundingBox"].setdefault(
                    "properties", {})
                if self.annotation_source:
                    src_properties["annotation_source"] = self.annotation_source
                if self.annotation_mode:
                    src_properties["annotation_mode"] = self.annotation_mode

            for ic in getattr(self, "_add_polygon_index", []):
                src_properties = x[0][ic]["AddPolygon"].setdefault(
                    "properties", {})
                if self.annotation_source:
                    src_properties["annotation_source"] = self.annotation_source
                if self.annotation_mode:
                    src_properties["annotation_mode"] = self.annotation_mode

Comment thread aperturedb/transformers/transformer.py Outdated
Comment thread aperturedb/transformers/common_properties.py Outdated
Comment thread aperturedb/transformers/bounding_box_properties.py Outdated
…tion

Removes cached add_bounding_box_index and add_polygon_index from the first transaction in Transformer, as these counts frequently vary per-item (e.g. COCO bounding boxes).

CommonProperties and BoundingBoxProperties now iterate over the current transactions commands directly, avoiding IndexError when subsequent items have fewer annotations, and ensuring proper property application when they have more.

Added a test case to explicitly check behavior with variable annotation counts.
Copilot AI review requested due to automatic review settings May 21, 2026 04:18
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Comment thread test/test_Transformers.py
Comment thread aperturedb/transformers/transformer.py
Comment thread aperturedb/transformers/video_properties.py
- Remove unused pytest import in test_Transformers.py
- Remove caching of AddImage and AddVideo indices in Transformer.__init__ to handle variable per-item counts
- Update CommonProperties and ImageProperties to scan for AddVideo dynamically instead of relying on cached indices
- Remove unused _blob_index_map from ImageProperties and VideoProperties
- Add unit test coverage for VideoProperties to ensure dynamic properties apply correctly
Copilot AI review requested due to automatic review settings May 21, 2026 10:31
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Comment thread aperturedb/transformers/transformer.py
Copilot AI review requested due to automatic review settings May 21, 2026 12:21
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Comment thread aperturedb/transformers/facenet_pytorch_embeddings.py Outdated
Comment thread aperturedb/transformers/clip_pytorch_embeddings.py Outdated
Comment thread test/test_Transformers.py Outdated
Copilot AI review requested due to automatic review settings May 22, 2026 18:54
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

aperturedb/transformers/clip_pytorch_embeddings.py:36

  • The descriptorset is only created if the first transaction contains an AddImage. If self.data[0] has no images (but later items do), this transformer will still append AddDescriptor commands in getitem, but may never create the descriptor set in the DB, causing ingestion failures. Consider scanning the first few items for an AddImage blob (or lazily initializing on first AddImage in getitem).
        # Let's sample some data to figure out the descriptorset we need.
        sample_blob = None
        for i, c in enumerate(self.data[0][0]):
            if list(c.keys())[0] == "AddImage":
                blob_idx = self._blob_index.index(i)
                sample_blob = self.data[0][1][blob_idx]
                break

        if sample_blob is not None:
            sample = generate_embedding(sample_blob)
            utils = self.get_utils()
            utils.add_descriptorset(
                self.search_set_name, dim=len(sample) // 4, metric=["CS"])

Comment thread aperturedb/transformers/facenet_pytorch_embeddings.py Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Comment thread test/test_Transformers.py Outdated
Copilot AI review requested due to automatic review settings May 25, 2026 17:47
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Copilot AI review requested due to automatic review settings May 25, 2026 22:04
@ad-claw000
Copy link
Copy Markdown
Contributor Author

Added tests for descriptor initialization retries to ensure new behavior is properly tested (see commit 339e593), addressing @luisremis's request for more testing on new features.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Comment thread test/test_Transformers.py
Copilot AI review requested due to automatic review settings May 26, 2026 13:22
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Comment thread aperturedb/transformers/common_properties.py
Comment thread aperturedb/transformers/bounding_box_properties.py
…hen all parameters are unset

Addresses review comments to avoid mutating command payloads with empty properties dicts when no annotations/common properties are provided.
Copilot AI review requested due to automatic review settings May 27, 2026 00:33
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.

Comment thread aperturedb/transformers/clip_pytorch_embeddings.py Outdated
Comment thread aperturedb/transformers/facenet_pytorch_embeddings.py Outdated
Comment thread test/run_test_container.sh Outdated
Comment thread test/run_test_container.sh Outdated
Copilot AI review requested due to automatic review settings May 27, 2026 15:09
@ad-claw000
Copy link
Copy Markdown
Contributor Author

Added more explicit assertions to test_Transformers.py to verify that AddDescriptor commands are correctly omitted when descriptor set initialization fails, addressing @luisremis's request for thorough testing of the new features. See commit a5df6fd.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add transformers for videos , bounding boxes

3 participants