Skip to content
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

Message improvements (properties and methods). #124

Merged
merged 4 commits into from
Nov 13, 2024

Conversation

kkeroo
Copy link
Collaborator

@kkeroo kkeroo commented Nov 6, 2024

This PR improves 3 messages:

  1. Predictions msg - we add a property prediction for retrieving first prediction value. This is useful because Regression parser many times returns one value and to get the value we must type message.predictions[0].prediction. Now, we will just write message.prediction.
  2. Classification msg - same idea as with Predictions msg. to get the most probable class and score we add two properties: top_class and top_score.
  3. ImgDetectionExtended msg - two methods are added for easy conversion to [x1,y1,x2,y2] bbox and for [[x1,y1], [x2,y2],[x3,y3],[x4,y4]] rotated bbox (going from the top-left and clockwise)

@github-actions github-actions bot added the messages Changes affecting ml.messages label Nov 6, 2024
Copy link

github-actions bot commented Nov 6, 2024

Test Results

125 tests   125 ✅  1s ⏱️
  1 suites    0 💤
  1 files      0 ❌

Results for commit c90782e.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Nov 6, 2024

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
3757 1256 33% 0% 🟢

New Files

No new covered files...

Modified Files

File Coverage Status
depthai_nodes/ml/messages/classification.py 81% 🟢
depthai_nodes/ml/messages/img_detections.py 67% 🟢
depthai_nodes/ml/messages/prediction.py 87% 🟢
TOTAL 79% 🟢

updated for commit: c90782e by action🐍

Copy link
Collaborator

@klemen1999 klemen1999 left a comment

Choose a reason for hiding this comment

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

LGTM

depthai_nodes/ml/messages/img_detections.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@jkbmrz jkbmrz left a comment

Choose a reason for hiding this comment

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

LGTM. The added features seem useful! I'm wondering, though, if we are not making the messages too complicated for DAI to implement natively (e.g. for bbox-related methods, wouldn't it be much easier for DAI to have such transformations done on-host?)

@kkeroo
Copy link
Collaborator Author

kkeroo commented Nov 11, 2024

LGTM. The added features seem useful! I'm wondering, though, if we are not making the messages too complicated for DAI to implement natively (e.g. for bbox-related methods, wouldn't it be much easier for DAI to have such transformations done on-host?)

Good question but IIRC Matevz said that we can add functions to msgs with no problem.

@aljazkonec1 aljazkonec1 merged commit 998ee80 into main Nov 13, 2024
11 checks passed
@aljazkonec1 aljazkonec1 deleted the improvement/messages branch November 13, 2024 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
messages Changes affecting ml.messages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants