Skip to content

Feat/annotation builder#178

Merged
tadeas0 merged 8 commits intomainfrom
feat/annotation_builder
Apr 16, 2025
Merged

Feat/annotation builder#178
tadeas0 merged 8 commits intomainfrom
feat/annotation_builder

Conversation

@tadeas0
Copy link
Contributor

@tadeas0 tadeas0 commented Feb 27, 2025

Purpose

Create AnnotationHelper class to simplify dai.ImgAnnotations message creation.

Dependencies & Potential Impact

None / not applicable

Deployment Plan

None / not applicable

Testing & Validation

None / not applicable

@tadeas0 tadeas0 requested a review from dominik737 February 27, 2025 11:36
@github-actions github-actions bot added the enhancement New feature or request label Feb 27, 2025
@codecov-commenter
Copy link

codecov-commenter commented Feb 27, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 85.86957% with 13 lines in your changes missing coverage. Please review.

Project coverage is 39.36%. Comparing base (dc06ae2) to head (91d3dcb).
Report is 23 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
depthai_nodes/utils/annotation_helper.py 85.86% 13 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #178      +/-   ##
==========================================
+ Coverage   38.36%   39.36%   +1.00%     
==========================================
  Files          78       79       +1     
  Lines        4309     4402      +93     
==========================================
+ Hits         1653     1733      +80     
- Misses       2656     2669      +13

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

dominik737
dominik737 approved these changes Feb 27, 2025
View reviewed changes
Copy link
Contributor

@dominik737 dominik737 left a comment

Choose a reason for hiding this comment

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

Looks great.

depthai_nodes/ml/helpers/utils/annotation_helper.py
ColorRGBA = Tuple[float, float, float, float]


class AnnotationHelper:
Copy link
Contributor

@dominik737 dominik737 Feb 27, 2025

Choose a reason for hiding this comment

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

I think the name of this class used to be AnnotationBuilder, why was it replaced for AnnotationHelper? I think that AnnotationBuilder is more suiting name.

Copy link
Contributor

@jkbmrz jkbmrz Feb 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Also, why not an ImgAnnotationBuilder if it refers to dai.ImgAnnotation creation?

Copy link
Contributor Author

@tadeas0 tadeas0 Feb 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

The name was changed after some discussion in this slack thread. Personally I don't have any strong opinions on the name of this class, so we can go with whatever you prefer.

@dominik737
Copy link
Contributor

dominik737 commented Feb 27, 2025 •
edited
Loading

Also I would consider moving the class from depthai-nodes.ml.helpers.utils to depthai-nodes.utils as the class does not directly relate to ml. And have the class in the __init__ file.

kkeroo
kkeroo approved these changes Feb 27, 2025
View reviewed changes
Copy link
Contributor

@kkeroo kkeroo left a comment

Choose a reason for hiding this comment

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

LGTM

jkbmrz
jkbmrz approved these changes Feb 28, 2025
View reviewed changes
depthai_nodes/ml/helpers/utils/annotation_helper.py
ColorRGBA = Tuple[float, float, float, float]


class AnnotationHelper:
Copy link
Contributor

@jkbmrz jkbmrz Feb 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Also, why not an ImgAnnotationBuilder if it refers to dai.ImgAnnotation creation?

@jkbmrz jkbmrz mentioned this pull request Apr 3, 2025
Merged
@dominik737
Copy link
Contributor

dominik737 commented Apr 7, 2025

Let's merge this when DepthAI alpha15 comes out. It should feature the bug fix for failing Windows tests.

@dominik737 dominik737 mentioned this pull request Apr 8, 2025
Merged
@dominik737
Copy link
Contributor

dominik737 commented Apr 15, 2025

Alpha15 is out. Please bump up the DepthAI version and merge this PR.

@tadeas0 tadeas0 merged commit 1094227 into main Apr 16, 2025
10 of 12 checks passed
@tadeas0 tadeas0 deleted the feat/annotation_builder branch April 16, 2025 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dominik737 dominik737 dominik737 approved these changes

@tersekmatija tersekmatija Awaiting requested review from tersekmatija tersekmatija is a code owner

@klemen1999 klemen1999 Awaiting requested review from klemen1999 klemen1999 is a code owner

+2 more reviewers

@kkeroo kkeroo kkeroo approved these changes

@jkbmrz jkbmrz jkbmrz approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@tadeas0 tadeas0

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@tadeas0 @codecov-commenter @dominik737 @kkeroo @jkbmrz