supervision icon indicating copy to clipboard operation
supervision copied to clipboard

Published 20 hours ago • verified publisher icon roboflow

Dropped frame_resolution_wh from [PolygonZone]

Open jeslinpjames opened this issue 1 year ago • 3 comments

Description

Issue #1101

Type of change

Please delete options that are not relevant.

  • [x] New feature (non-breaking change which adds functionality)
  • [x] This change requires a documentation update

How has this change been tested, please provide a testcase or example of how you tested the change?

Here is the Colab link with the changes made to the PolygonZone Class.

Any specific deployment considerations

For example, documentation changes, usability, usage/costs, secrets, etc.

Docs

  • [x] Docs updated? What were the changes:
  • Removed frame_resolution_wh from the documentation.

jeslinpjames avatar Apr 11 '24 13:04 jeslinpjames

Many examples in the repository still pass frame_resolution_wh as a parameter when using PolygonZone. Considering its deprecation, shall we update these examples for consistency? I'm happy to proceed with the changes if deemed appropriate.

jeslinpjames avatar Apr 11 '24 14:04 jeslinpjames

Many examples in the repository still pass frame_resolution_wh as a parameter when using PolygonZone. Considering its deprecation, shall we update these examples for consistency? I'm happy to proceed with the changes if deemed appropriate.

Let's fix the test errors first.

onuralpszr avatar Apr 11 '24 18:04 onuralpszr

Hi @onuralpszr,

I handle the initial test error but there are still failures in the pytest suite. After investigating the failures, I found that the frame_resolution_wh parameter, used for clipping bounding boxes in the trigger function, leads to errors as it expects the resolution of the video. The clipped_xyxy calculation within the trigger function clips the bounding boxes within the polygon, resulting in incorrect anchor point detection. This discrepancy causes test failures as shown in the traceback.

So, how should we proceed?

jeslinpjames avatar Apr 12 '24 13:04 jeslinpjames

Hi @jeslinpjames and @onuralpszr, I just fixed the tests. We are ready to merge!

SkalskiP avatar Apr 12 '24 14:04 SkalskiP

Many examples in the repository still pass frame_resolution_wh as a parameter when using PolygonZone. Considering its deprecation, shall we update these examples for consistency? I'm happy to proceed with the changes if deemed appropriate.

Hey @SkalskiP, should I put another PR with the examples changed?

jeslinpjames avatar Apr 12 '24 14:04 jeslinpjames

Many examples in the repository still pass frame_resolution_wh as a parameter when using PolygonZone. Considering its deprecation, shall we update these examples for consistency? I'm happy to proceed with the changes if deemed appropriate.

Hey @SkalskiP, should I put another PR with the examples changed?

Please do so :)

onuralpszr avatar Apr 12 '24 14:04 onuralpszr

But please test examples and their behavior as well. If dropping creates side effects, it would be nice to know it

onuralpszr avatar Apr 12 '24 14:04 onuralpszr

But please test examples and their behavior as well. If dropping creates side effects, it would be nice to know it

Will do.

jeslinpjames avatar Apr 12 '24 14:04 jeslinpjames

@jeslinpjames yup, please open second PR with changed examples. I fixed one bug when I fixed tests. So I don't expect behavior problems. But test anyway:)

SkalskiP avatar Apr 12 '24 16:04 SkalskiP

@jeslinpjames yup, please open second PR with changed examples. I fixed one bug when I fixed tests. So I don't expect behavior problems. But test anyway:)

Hey, I tested out a couple of examples, and everything looks good. I'll put another PR soon.

jeslinpjames avatar Apr 13 '24 03:04 jeslinpjames