Skip to content

feat: generate_script_content update #206

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

Merged
merged 6 commits into from
Apr 22, 2025
Merged

feat: generate_script_content update #206

merged 6 commits into from
Apr 22, 2025

Conversation

jkbmrz
Copy link
Collaborator

@jkbmrz jkbmrz commented Apr 17, 2025

Purpose

Platform differentiation in the generate_script_content function is not necessary anymore because we can now utilize ImageManipConfigV2 both for RVC2 and RVC4.

Specification

  • update generate_script_content function:
    • remove platform differentiation during cfg_content creation,
    • add resize_mode setting;
  • update associated unittests in the unittests/test_nodes/test_host_nodes/test_detection_config_generator.py na rename it to test_generate_script_content.py

Dependencies & Potential Impact

Removing the platform parameter from the generate_script_content function will cause some disturbances. Thankfully, none of the examples in depthai-experiments imports this function (but they rather use custom implementations).

Deployment Plan

None

Testing & Validation

Running the updated human-pose experiment (HERE) on RVC2.

Copy link
Collaborator

@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.

@luxonis luxonis deleted a comment from codecov-commenter Apr 18, 2025
@codecov-commenter
Copy link

codecov-commenter commented Apr 18, 2025

Codecov Report

Attention: Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 53.99%. Comparing base (1094227) to head (0695f81).
Report is 1 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...hai_nodes/node/utils/detection_config_generator.py 66.66% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main     #206       +/-   ##
===========================================
+ Coverage   31.84%   53.99%   +22.14%     
===========================================
  Files          53       85       +32     
  Lines        3074     5078     +2004     
===========================================
+ Hits          979     2742     +1763     
- Misses       2095     2336      +241     

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

@jkbmrz
Copy link
Collaborator Author

jkbmrz commented Apr 18, 2025

The test_detection_config_generator.py is named a bit awkwardly. Could we rename it to test_generate_script_content.py? This seems more clear and also follows the naming convention we use elsewhere (test_<class/function_name>)

@jkbmrz jkbmrz marked this pull request as ready for review April 18, 2025 13:01
@kkeroo
Copy link
Collaborator

kkeroo commented Apr 18, 2025

Agree.

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

@jkbmrz jkbmrz merged commit a6ad5b7 into main Apr 22, 2025
12 checks passed
@jkbmrz jkbmrz deleted the fix/config_generator branch April 22, 2025 08:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Fixing a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants