Fix: Make ApertureDBDataset applicable to Videos as well - #726
Conversation
There was a problem hiding this comment.
Pull request overview
Generalizes ApertureDBDataset (PyTorch integration) so it can iterate over blob-returning Find* commands beyond FindImage, enabling use cases like FindVideo while only applying OpenCV decoding for image blobs.
Changes:
- Detects a
Find*command dynamically and uses it for batched retrieval (batch+blobs). - Returns raw blobs unmodified for non-
FindImagequeries; keepscv2decode path forFindImage. - Updates dataset docstring/field naming to reflect broader applicability.
Comments suppressed due to low confidence (1)
aperturedb/PyTorchDataset.py:46
- The error message says the query "must contain one Find command", but the code currently accepts multiple Find* commands (and will choose one implicitly). Please either validate that there is exactly one eligible Find* command, or update the message to explain how the command is chosen (and which Find* commands are supported).
if self.command_idx is None:
logger.error(
"Query error. The query must contain one Find command")
raise Exception('Query Error')
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
aperturedb/PyTorchDataset.py:67
- This
raise Exception(...)f-string is also split across lines inside{ ... }, which is a SyntaxError. Build the message without breaking the f-string expression across lines (e.g., keep the interpolation on one line or precompute the pieces).
resp = r[self.command_idx][self.command_name]
if resp.get("status", 0) != 0:
raise Exception(f"Query Error: {resp.get('status')} {
resp.get('info', '')}")
batch = resp["batch"]
|
Done. Updated the error message to clarify how Find* commands are handled, as suggested. |
|
I have merged develop and resolved the conflicts. The PR is ready for another look @luisremis. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (2)
aperturedb/PyTorchDataset.py:77
- The loop that disables blob-returning behavior currently matches any command whose name starts with
"Find". This will also add a"blobs"field to non-blob commands likeFindEntity, which may make the query invalid if the server rejects unknown fields. It’s safer to only setblobson known blob-returning commands.
for i in range(len(self.query)):
name = list(self.query[i].keys())[0]
if name.startswith("Find") and i != self.command_idx:
self.query[i][name]["blobs"] = False
aperturedb/TensorFlowDataset.py:75
- The loop that disables blob-returning behavior currently matches any command whose name starts with
"Find". This will also add a"blobs"field to non-blob commands likeFindEntity, which may make the query invalid if the server rejects unknown fields. It’s safer to only setblobson known blob-returning commands.
for i in range(len(self.query)):
name = list(self.query[i].keys())[0]
if name.startswith("Find") and i != self.command_idx:
self.query[i][name]["blobs"] = False
- Add status check to TensorFlowDataset.py inference logic. - Add mock FindVideo tests to torch and tf connectors.
Summary
PyTorchDataset.pygeneral enough to handleFindVideo(and otherFind*commands) along withFindImage.FindImagequery, wherecv2image decoding still safely applies.Verification
PyTorchDataset.py.Fixes #168