-
Notifications
You must be signed in to change notification settings - Fork 1
[RSDK-9506] explicitly use xMin, yMin, etc. in bounding boxes #7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you create unit tests for 3 cases:
- An empty valid regions attribute
- a correct valid regions attribute
- an incorrect valid regions attribute
You can use the struct directly, you dont have to do the json parsing
| CountPeriod float64 `json:"sampling_period_s"` | ||
| NSamples int `json:"n_samples"` | ||
| ExtraFields map[string]interface{} `json:"extra_fields"` | ||
| ValidRegions map[string][]BoundingBoxConfig `json:"valid_regions"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You'll def want to manually test this on a local machine to see if it works as intended
000b768 to
22a38fd
Compare
|
Take another look. I've added a bunch of unit tests, and they all pass. I've also gotten this up and working with a webcam on my desk: It reads "none" when I duck into the bottom half of the frame, and "one" when I sit up straight in the top half. I can change the Y range to the full 0-1 and the X range to 0-0.5, and it reads "one" when I'm in the left half and "none" when I'm in the right half. Finally, if I set XMax and XMin both to 0, it reads "one" no matter where in the frame I am (and "none" if I'm out of frame) |
bhaney
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
* update readme per PR #7 * tweak the text, too
Everything compiles, but I haven't tried running it yet. I'll do that while you take a look at the changes...
(doing this now because Alexis got confused why a ticket assigned to me was in her epic)