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

[Copilot]

Pull request overview

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

Suppressed comments (1)

aperturedb/DataModels.py:7

• Optional is imported but not used anywhere in this module, which adds noise and may trip linters/static checks. Remove it from the import list.

from typing import ClassVar, Optional

[Copilot]

Pull request overview

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

[ad-claw000]

Addressed feedback from @luisremis:

• Added testing for FrameDataCSV in test_FrameDataCSV.py.
• Added Frames to the Object Model (analogous to Images and Videos) and added corresponding tests in test_Frames.py.

Updated in commit f393910

[ad-claw000]

Addressed feedback from @copilot-pull-request-reviewer[bot]:

• Fixed the autopep8 line formatting issues in test_FrameDataCSV.py and Frames.py.
• Reverted the url override in FrameDataModel to avoid Pydantic field redefinition errors.

Updated in commits f393910 and 17ee930

[Copilot]

Pull request overview

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

Suppressed comments (1)

aperturedb/Images.py:1011

• The __getattr__ shim makes from aperturedb.Images import Frames work again, but from aperturedb.Images import * will no longer export Frames because the name isn’t in the module namespace. If the intent is full backward compatibility, add a module __all__ that mirrors the default export set and appends Frames, letting import-star retrieve it via __getattr__ without reintroducing the circular import.

def __getattr__(name: str):
    if name == "Frames":
        from aperturedb.Frames import Frames
        return Frames
    raise AttributeError(f"module {__name__!r} has no attribute {name!r}")

[ad-claw000]

Addressed the latest Copilot reviewer comment in commit c560a56 by explicitly defining __all__ in Images.py so that Frames is exported during import *.

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

Suppressed comments (1)

aperturedb/Images.py:1011

• __getattr__ returns Frames but doesn’t cache it into the module namespace. This means every Images.Frames access re-runs the import lookup and dir(aperturedb.Images) won’t show Frames, which is a small but real backward-compatibility regression compared to the previous in-module Frames class. Cache the resolved class in globals() before returning it (same pattern used in aperturedb/transformers/__init__.py).

def __getattr__(name: str):
    if name == "Frames":
        from aperturedb.Frames import Frames
        return Frames
    raise AttributeError(f"module {__name__!r} has no attribute {name!r}")

[ad-claw000]

Addressed the latest Copilot reviewer comment in commit 83878fd by caching the lazily-imported Frames class in globals() to prevent repeated lookups and properly expose it in the module namespace.

[Copilot]

Pull request overview

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

Suppressed comments (4)

aperturedb/Images.py:1012

• The backward-compatibility behavior is now implemented via module-level __getattr__, but there’s no test covering the previously supported import path (e.g., from aperturedb.Images import Frames). Add a test that imports Frames from aperturedb.Images and asserts it resolves to aperturedb.Frames.Frames (and ideally that it’s cached on the module after first access) to prevent regressions.

# 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}")

aperturedb/FrameDataCSV.py:16

• The entity name is hard-coded as \"_Frame\". To avoid drift if naming/constants change elsewhere (and to align with the rest of the code using ObjectType), prefer deriving this from the shared enum/constant (e.g., ObjectType.FRAME.value) rather than duplicating the string literal.

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

aperturedb/DataModels.py:75

• Changing FrameDataModel’s base class from IdentityDataModel to BlobDataModel changes the model’s inherited fields/validation and can break callers constructing FrameDataModel with the old schema. Please document the expected fields for frames (now that it’s blob-backed) and add/adjust a unit test that validates the accepted/required fields for FrameDataModel to lock down the intended public contract.

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

test/test_FrameDataCSV.py:25

• Calling f.close() inside a with NamedTemporaryFile(...) as f: block is redundant (the context manager will close the file). Removing the explicit close improves readability and avoids confusion about file lifecycle.

    with tempfile.NamedTemporaryFile(
        suffix=".csv", mode="w", delete=False
    ) as f:
        f.write("url,id\nhttp://example.com/frame.jpg,1\n")
        f.close()

[ad-claw000]

Addressed the latest review feedback in commit 087f510:

• Updated FrameDataCSV to use ObjectType.FRAME.value.
• Added backward compatibility import test for Frames.
• Documented FrameDataModel and added a test for its required fields.
• Cleaned up redundant file closure in test_FrameDataCSV.py.

[Copilot]

Pull request overview

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

Suppressed comments (2)

aperturedb/ImageDataCSV.py:202

• getattr(self, "command", "AddImage") will preserve an explicitly-set command=None (or empty string), resulting in an invalid command string later. Consider falling back when the retrieved value is falsy (e.g., use "AddImage" when the subclass/instance didn’t provide a usable command). This keeps the new subclass-override behavior while avoiding None leaking into ingestion logic.

        self.relative_path_prefix = os.path.dirname(self.filename) \
            if self.source_type == HEADER_PATH and self.blobs_relative_to_csv else ""

        self.command = getattr(self, "command", "AddImage")

aperturedb/Images.py:1015

• Building __all__ dynamically from globals() can unintentionally export internal/imported symbols and can also produce duplicate entries (e.g., "Frames" could appear twice after it’s cached into globals()). Prefer defining __all__ explicitly (or deduplicating and limiting it to the intended public API) so from aperturedb.Images import * remains stable and doesn’t accidentally surface non-API names.

# 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}")


__all__ = [k for k in globals().keys() if not k.startswith('_')] + ['Frames']