feat: Add CSV ingestion support for Frames

[ad-claw000]

Closes #70

This PR adds a FrameDataCSV class which simply inherits from ImageDataCSV and overrides the command to AddFrame. It also registers IngestType.FRAME in the CLI, closing the loop on Frame OM and ingestion support without duplicating the complex loading/validation logic of Images.

[Copilot]

Pull request overview

This PR extends the Python SDK’s CSV ingestion pipeline to support Frame objects by introducing a FrameDataCSV adapter over the existing image ingestion logic, and wiring the new ingest type into the CLI.

Changes:

• Allow ImageDataCSV subclasses to override the command used in CSVParser-generated queries (AddImage vs AddFrame).
• Add FrameDataCSV for frame ingestion via CSV.
• Register IngestType.FRAME in the adb ingest from-csv CLI mapping.

Reviewed changes

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

File	Description
aperturedb/ImageDataCSV.py	Makes the ingestion command overrideable (enables reuse for Frames).
aperturedb/FrameDataCSV.py	Adds a new CSV ingester for Frames (needs _Frame index override).
aperturedb/cli/ingest.py	Registers FRAME ingest type in CLI (currently missing BBoxDataCSV import).

Suppressed comments (1)

aperturedb/cli/ingest.py:205

• BBoxDataCSV is referenced in ingest_types but is no longer imported in this function, which will raise NameError when from_csv runs (even for non-bounding-box ingest types, since the dict is constructed unconditionally). Re-add the missing import.

    from aperturedb.ImageDataCSV import ImageDataCSV
    from aperturedb.FrameDataCSV import FrameDataCSV
    from aperturedb.EntityDataCSV import EntityDataCSV
    from aperturedb.BlobDataCSV import BlobDataCSV
    from aperturedb.ConnectionDataCSV import ConnectionDataCSV
    from aperturedb.PolygonDataCSV import PolygonDataCSV
    from aperturedb.VideoDataCSV import VideoDataCSV
    from aperturedb.DescriptorDataCSV import DescriptorDataCSV
    from aperturedb.DescriptorSetDataCSV import DescriptorSetDataCSV

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[luisremis]

add testing to the new FrameDataCSV.
add Frame to the OM, analogous to how Image and Video work.

[Copilot]

Pull request overview

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

Suppressed comments (2)

aperturedb/cli/ingest.py:199

• from_csv() builds the ingest_types dict using BBoxDataCSV (for IngestType.BOUNDING_BOX) but the import was removed. This will raise a NameError when invoking adb ingest from-csv with --ingest-type BOUNDING_BOX. Re-add the missing import alongside the other CSV parsers.

    from aperturedb.ImageDataCSV import ImageDataCSV
    from aperturedb.FrameDataCSV import FrameDataCSV
    from aperturedb.EntityDataCSV import EntityDataCSV
    from aperturedb.BlobDataCSV import BlobDataCSV

aperturedb/FrameDataCSV.py:14

• New FrameDataCSV behavior (overridden command + _Frame indices) is not covered by tests. There are existing tests exercising other CSV ingesters (e.g., test/test_SPARQL.py uses ImageDataCSV and EntityDataCSV), so adding at least a small unit test for FrameDataCSV would help prevent regressions (e.g., verifying command == "AddFrame" and get_indices() targets _Frame).

class FrameDataCSV(ImageDataCSV):
    def __init__(self, *args, **kwargs):
        self.command = "AddFrame"
        super().__init__(*args, **kwargs)

    def get_indices(self):
        return {
            "entity": {
                "_Frame": self.get_indexed_properties()
            }
        }

[Copilot]

Pull request overview

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

Suppressed comments (3)

aperturedb/Images.py:1004

• Frames used to be defined in aperturedb/Images.py; removing it from this module is a breaking change for any code that does from aperturedb.Images import Frames. If the intent is just to move the implementation, consider adding a backwards-compatible shim (e.g., module __getattr__) that resolves Frames lazily from aperturedb.Frames.

        return return_dictionary

test/test_FrameDataCSV.py:6

• pytest and pandas are imported but never used in this test, which adds unnecessary dependencies and can trigger unused-import lint failures.

import pytest
import pandas as pd
import tempfile
import os
from aperturedb.FrameDataCSV import FrameDataCSV

aperturedb/FrameDataCSV.py:8

• FrameDataCSV only needs to override the command string; defining a variadic __init__ drops the base-class signature and makes introspection/type checking harder. With ImageDataCSV now using getattr(self, "command", ...), you can set command as a class attribute and remove the custom initializer.

class FrameDataCSV(ImageDataCSV):
    def __init__(self, *args, **kwargs):
        self.command = "AddFrame"
        super().__init__(*args, **kwargs)

[Copilot]

Pull request overview

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

Suppressed comments (3)

aperturedb/Images.py:1004

• Frames used to be defined in this module; removing it breaks existing user code that imports Frames via from aperturedb.Images import Frames. Consider re-exporting the new implementation from this module to preserve backward compatibility while keeping the real class in aperturedb/Frames.py.

        return return_dictionary

aperturedb/FrameDataCSV.py:8

• FrameDataCSV is a new public CSV-ingestion helper but it lacks the class-level docstring that other *DataCSV classes provide (e.g. BBoxDataCSV). Adding a short docstring helps generated docs and keeps the module consistent.

class FrameDataCSV(ImageDataCSV):
    def __init__(self, *args, **kwargs):
        self.command = "AddFrame"
        super().__init__(*args, **kwargs)

test/test_FrameDataCSV.py:5

• pytest and pandas are imported but never used in this test module; removing unused imports keeps the test lightweight and avoids unnecessary dependency coupling.

import pytest
import pandas as pd
import tempfile
import os
from aperturedb.FrameDataCSV import FrameDataCSV

[ad-claw000]

Addressed the latest Copilot reviewer comments in commit cce1bc7 (added backward-compatibility shim for Frames in Images.py, added docstring to FrameDataCSV, and removed unused imports in the test module).

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

Suppressed comments (3)

test/test_FrameDataCSV.py:7

• This line is long enough that autopep8 will likely reformat it; in this repo CI enforces autopep8 formatting, so it’s better to wrap it now to avoid formatting-only CI failures.

    with tempfile.NamedTemporaryFile(suffix=".csv", mode="w", delete=False) as f:

aperturedb/Frames.py:21

• This super().__init__ call exceeds typical autopep8 line wrapping; wrapping it avoids formatting-only CI failures and keeps it consistent with the rest of the codebase’s style.

    def __init__(self, client, batch_size=100, response=None, **kwargs):
        super().__init__(client, batch_size=batch_size, response=response, **kwargs)

aperturedb/DataModels.py:76

• Changing FrameDataModel from IdentityDataModel to BlobDataModel makes url a required field (via BlobDataModel). If any callers construct FrameDataModel without a URL today, this is a breaking API change. If this is intended, it should be called out in the PR description/changelog; if not intended, consider keeping IdentityDataModel or making url optional on FrameDataModel.

class FrameDataModel(BlobDataModel):
    """Frame data model for ApertureDB.
    """
    type = ObjectType.FRAME

[ad-claw000]

Addressed the latest suppressed Copilot review comments in commit ec043dc. Replaced hardcoded command lists with shared constants (PROPERTY_ADD_COMMANDS, BLOB_ADD_COMMANDS) in common_properties.py and MLCroissant.py, and renamed nested side_effect functions in the connector tests to improve traceback clarity.

[Copilot]

Pull request overview

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

Suppressed comments (6)

test/test_torch_connector.py:200

• The test uses an all-zero image, which cannot validate that FindFrame decoding + BGR→RGB conversion is actually happening (the output will look identical even if channel order is wrong). Use a non-symmetric pixel pattern (e.g., set one pixel to a known BGR value) and assert the decoded numpy array matches the expected RGB value to meaningfully cover the new decode path.

                img = np.zeros((10, 10, 3), dtype=np.uint8)
                is_success, buffer = cv2.imencode(".jpg", img)
                assert is_success, "Failed to encode image"
                b = [buffer.tobytes()]

test/test_torch_connector.py:210

• The test uses an all-zero image, which cannot validate that FindFrame decoding + BGR→RGB conversion is actually happening (the output will look identical even if channel order is wrong). Use a non-symmetric pixel pattern (e.g., set one pixel to a known BGR value) and assert the decoded numpy array matches the expected RGB value to meaningfully cover the new decode path.

                assert isinstance(blob, np.ndarray)
                assert blob.shape == (10, 10, 3)

test/test_tf_connector.py:228

• Similar to the PyTorch test, using an all-zero image doesn’t verify that the OpenCV decode + color conversion branch is correct for FindFrame (BGR vs RGB won’t be detectable). Consider encoding an image with a distinctive channel value and asserting a specific pixel’s RGB values in the produced tensor/array.

                img = np.zeros((10, 10, 3), dtype=np.uint8)
                is_success, b_img = cv2.imencode('.jpg', img)
                assert is_success, "Failed to encode image"
                b = [b_img.tobytes()]

test/test_tf_connector.py:242

• Similar to the PyTorch test, using an all-zero image doesn’t verify that the OpenCV decode + color conversion branch is correct for FindFrame (BGR vs RGB won’t be detectable). Consider encoding an image with a distinctive channel value and asserting a specific pixel’s RGB values in the produced tensor/array.

                assert data.shape == (10, 10, 3)

aperturedb/MLCroissant.py:262

• Importing BLOB_ADD_COMMANDS inside getitem adds per-call overhead on a hot path (even though the module import is cached). Prefer a module-level import for consistency with the other transformers, unless there is a concrete circular-import constraint that requires the local import.

        from aperturedb.Constants import BLOB_ADD_COMMANDS

aperturedb/MLCroissant.py:277

• Importing BLOB_ADD_COMMANDS inside getitem adds per-call overhead on a hot path (even though the module import is cached). Prefer a module-level import for consistency with the other transformers, unless there is a concrete circular-import constraint that requires the local import.

            if cmd in BLOB_ADD_COMMANDS:

[ad-claw000]

Addressed the latest suppressed Copilot review comments in commit e78cb1d.

• test_torch_connector.py & test_tf_connector.py: Updated test_find_frame_mocked to encode a non-symmetric pixel (blue) using .png and asserted that it decodes to RGB (red) to verify the new color conversion path.
• MLCroissant.py: Moved BLOB_ADD_COMMANDS import to the module level to eliminate per-call overhead in getitem.

[Copilot]

Pull request overview

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

Suppressed comments (1)

aperturedb/MLCroissant.py:1

• Switching from an explicit list ([\"AddImage\", \"AddBlob\", \"AddVideo\"]) to BLOB_ADD_COMMANDS broadens the skip behavior (notably it now also skips AddDescriptor and AddFrame). If MLCroissant.getitem() is intended to skip only a subset of blob add commands (as the previous code implied), introduce a more specific constant for this context (e.g., a dedicated set for 'commands excluded from index creation') or keep a local explicit set that preserves the prior semantics and add AddFrame only if desired.

import dataclasses

[ad-claw000]

Addressed the final suppressed Copilot review comment in commit ed0c692:

• MLCroissant.py: Reverted to a local explicit skip-list for Croissant indexing to preserve prior semantics without broadly skipping all BLOB_ADD_COMMANDS.

[Copilot]

Pull request overview

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

Suppressed comments (1)

aperturedb/MLCroissant.py:24

• This introduces yet another hardcoded command-name set that substantially overlaps with the new shared constants in aperturedb.Constants (e.g., BLOB_ADD_COMMANDS). To prevent future drift, consider deriving this from a shared constant (or moving a Croissant-specific constant into Constants.py). If AddDescriptor is intentionally excluded here, adding a brief comment explaining why would also help keep the intent clear.

# Commands for which we do not want to automatically create entity indices
_CROISSANT_SKIP_INDEX_COMMANDS = frozenset([
    "AddImage",
    "AddBlob",
    "AddVideo",
    "AddFrame"
])

[ad-claw000]

Addressed the latest suppressed Copilot review comment in commit c5bdd66:

• MLCroissant.py: Derived _CROISSANT_SKIP_INDEX_COMMANDS from BLOB_ADD_COMMANDS and explicitly documented why AddDescriptor is excluded.

[Copilot]

Pull request overview

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

Suppressed comments (3)

aperturedb/Images.py:1019

• The lazy __getattr__ shim preserves from aperturedb.Images import Frames, but it no longer preserves from aperturedb.Images import * behavior because Frames is not present in module.__dict__ at import time. To keep backward compatibility, define __all__ to include Frames (so star-import will call getattr for it), or eagerly populate Frames in globals() at import time (trading away lazy import).

# Shim for backward compatibility
def __getattr__(name: str):
    if name == "Frames":
        from aperturedb.Frames import Frames
        globals()[name] = Frames
        return Frames
    raise AttributeError(f"module {__name__!r} has no attribute {name!r}")


def __dir__():
    return sorted(set(list(globals().keys()) + ["Frames"]))

test/test_tf_connector.py:243

• In TensorFlow, data.shape is a TensorShape, and comparing it directly to a tuple can be brittle across TF versions/graph/eager contexts. Prefer asserting tuple(data.shape) == (10, 10, 3) or data.shape.as_list() == [10, 10, 3] to make the test robust.

            for data, label in dataset:
                assert data.shape == (10, 10, 3)
                assert np.array_equal(data[0, 0].numpy(), [

test/test_cli_ingest.py:20

• This test verifies that FrameDataCSV is constructed, but it doesn't verify that the constructed object is passed into _process_data. Strengthen the assertion by checking _process_data was called with the mocked data instance (and optionally that key parameters like sample_count are forwarded as expected).

            from_csv(filepath="dummy.csv",
                     ingest_type=IngestType.FRAME, sample_count=5)

            mock_csv_class.assert_called_once_with(
                "dummy.csv", use_dask=False, blobs_relative_to_csv=True)
            mock_process_data.assert_called_once()

[ad-claw000]

Addressed the latest suppressed Copilot review comments in commit 93ec0c5:

• Images.py: Replaced dynamic __getattr__/__dir__ with an eager import of Frames at the module's end. This naturally avoids the circular import while completely restoring the original behavior of from aperturedb.Images import * without needing __all__.
• test_tf_connector.py: Asserted against tuple(data.shape) instead of the raw TensorShape object to improve test robustness.
• test_cli_ingest.py: Strengthened the _process_data mock assertion to verify all keyword arguments (like sample_count) are correctly passed.

[Copilot]

Pull request overview

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

Suppressed comments (1)

aperturedb/DataModels.py:7

• Optional is imported but unused in this module, which adds noise and can trigger unused-import checks in some environments.

from typing import ClassVar, Optional

[Copilot]

Pull request overview

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

Suppressed comments (1)

aperturedb/DataModels.py:7

• Optional is imported but not used in this module, which adds noise and can confuse future edits. Remove it from the import list (or start using it if needed).

from typing import ClassVar, Optional

[ad-claw000]

Addressed the latest suppressed Copilot review comment in commit ab160d8:

• DataModels.py: Removed the unused Optional import.

[Copilot]

Pull request overview

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