Skip to content

fix: Update operations on DW Operations. - #688

Merged
luisremis merged 17 commits into
developfrom
fix/issue-160
May 23, 2026
Merged

fix: Update operations on DW Operations.#688
luisremis merged 17 commits into
developfrom
fix/issue-160

Conversation

@ad-claw000

@ad-claw000 ad-claw000 commented May 19, 2026

Copy link
Copy Markdown
Contributor

Summary

Updates Operations class with preview and threshold operations, and refactors resize.
Also includes CI and test infrastructure optimizations to improve reliability.

Changes

  • Added threshold operation.
  • Added preview operation.
  • Updated resize operation to accept width, height, and scale as optional arguments, with validation to reject mixed/empty args.
  • Bumped GitHub Actions checks job to use Python 3.12 to align with updated runtime dependencies.
  • Updated test container/docker-compose port publishing strategy to avoid port conflicts during concurrent CI runs.

Testing

  • Added test/test_Operations.py to verify old and new Operations configurations.
  • Verified test container changes locally.

Fixes #160

@luisremis luisremis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

add testing for all old and new operations

Copilot AI review requested due to automatic review settings May 19, 2026 21:01
@ad-claw000

Copy link
Copy Markdown
Contributor Author

Addressed the review comments by adding test/test_Operations.py with tests for both old and new operations, including chained operations.

luisremis
luisremis previously approved these changes May 19, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the aperturedb.Operations.Operations builder to support additional on-the-fly media operations and to broaden the resize API so it can be expressed using width, height, and/or scale. It also adds a focused unit test module to validate the generated operations payloads.

Changes:

  • Added threshold() operation builder.
  • Added preview() operation builder with optional limiting parameters.
  • Refactored resize() to accept optional width, height, and scale, and added tests for the operations array output.

Reviewed changes

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

File Description
aperturedb/Operations.py Extends the operations DSL with threshold and preview, and changes resize to accept optional sizing parameters.
test/test_Operations.py Adds unit tests asserting operation payload shapes for resize/rotate/flip/crop/interval/threshold/preview and chaining.
Comments suppressed due to low confidence (1)

aperturedb/Operations.py:89

  • preview() arguments default to None but are annotated as non-optional types. Please change them to Optional[...] (or T | None) so the signature matches actual accepted values and aligns with typing style used elsewhere in the repo.
    def preview(self, max_frame_count: int = None, max_time_fraction: float = None, max_time_offset: str = None, max_size_mb: float = None) -> Operations:

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

Comment thread aperturedb/Operations.py Outdated
Comment thread aperturedb/Operations.py Outdated
Comment thread test/test_Operations.py

@ad-claw000 ad-claw000 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Applied autopep8 formatting to the new log messages to fix the pre-commit CI failure.

Copilot AI review requested due to automatic review settings May 20, 2026 02:58

@ad-claw000 ad-claw000 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed the remaining feedback about type hints and tests. PTAL!

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 18 out of 20 changed files in this pull request and generated 2 comments.

Comment thread aperturedb/Operations.py
Comment thread .github/workflows/checks.yml

@ad-claw000 ad-claw000 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed the feedback: updated Operations.resize to reject invalid scale vs width/height combinations, updated Images.resolve to correctly handle 'scale', and updated the PR description with the CI details.

Copilot AI review requested due to automatic review settings May 20, 2026 03:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 19 out of 21 changed files in this pull request and generated 2 comments.

Comment thread test/test_Operations.py Outdated
Comment thread aperturedb/Operations.py
@ad-claw000

Copy link
Copy Markdown
Contributor Author

I have reviewed the PR and verified that all actionable review comments have been properly addressed:

  • The resize() validation and coordinate-resolution logic handling for scale and missing dimensions were addressed in commit fde49d7 (aperturedb/Operations.py, aperturedb/Images.py).
  • Optional type annotations for preview() parameters were fixed in commit 23c49d2 (aperturedb/Operations.py).
  • Unused pytest import is now used for negative tests in commit 3d5be27 (test/test_Operations.py).
  • The PR description was correctly updated to reflect the Python 3.12 CI bump and docker-compose port publishing changes.
  • test_chained_operations and negative tests for invalid resize arguments were updated and added in commit 3d5be27 (test/test_Operations.py).
  • Testing for all old and new operations as requested by luisremis is covered in test/test_Operations.py.

Tests have been run locally and all 11 operations tests passed successfully. All review threads have been marked as resolved.

@ad-claw000 ad-claw000 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added testing for all old and new operations in test/test_Operations.py (commits 5fc0bb2 and 3d5be27). All operations are now covered.

@ad-claw000

Copy link
Copy Markdown
Contributor Author

I've also added a missing test for Images.resolve with resize scale in commit 1e60b94 (test_Images.py) to ensure the new scaling logic is fully covered.

Copilot AI review requested due to automatic review settings May 20, 2026 12:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 19 out of 21 changed files in this pull request and generated 3 comments.

Comment thread aperturedb/Images.py Outdated
Comment thread test/run_test_container.sh Outdated
Comment thread test/test_Images.py Outdated
@ad-claw000

Copy link
Copy Markdown
Contributor Author

I've added the missing tests for the remaining preview parameters and ignored operations in Images.resolve to fully address the review comment about testing old and new operations.

Copilot AI review requested due to automatic review settings May 21, 2026 06:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 18 out of 20 changed files in this pull request and generated no new comments.

@ad-claw000 ad-claw000 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added testing for all old and new operations in test/test_Operations.py (covered in recent commits, plus test_empty_operations in d830c30). All operations are now covered.

Copilot AI review requested due to automatic review settings May 23, 2026 01:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 4 out of 4 changed files in this pull request and generated no new comments.

@luisremis
luisremis merged commit d5e13e6 into develop May 23, 2026
2 checks passed
@luisremis
luisremis deleted the fix/issue-160 branch May 23, 2026 03:00
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.

Update operations on DW Operations.

3 participants