Conversation
|
Can you review this PR |
There was a problem hiding this comment.
Hello @Chandraveersingh1717,
Thanks for this proposal.
todo Could you please remove from this proposal all file changes unrelated to Keypoint skeleton ?
(i.e. do not stack changes on top of each other, but create new branch from a clean main each time)
todo Please run devtools::check() and devtools::document() before submitting P.R. to the package (here some Rd file changes are missing, and exported function documentation miss). You then have to fix each and every Error, Warning and Note related to your changes.
todo Please do not remove documentation from exported functions. (as it would generate a check error) because documentation is the first-class asset of your contribution.
suggestion Please avoid to lint the code you are not editing. It would create a mess in other people contribution review.
suggestion Please remove generated lines of "=" sign in the code base.
I'll be able to make a cleaner review after that
Thanks
There was a problem hiding this comment.
praise Fully annotated human pose is very nice idea, thanks
suggestion please choose a more contrastive color with the image for accessibility.
todo please review the points coordinate so that result is less modern-art human pose :
- Implement drawing lines between keypoints using connectivity matrix - Add graphics::segments() to draw skeleton connections - Update documentation with connectivity parameter and examples - Add test for connectivity feature - Add keypoint-skeleton.R example for human pose visualization
Add connectivity parameter to draw lines between keypoints with human pose example
Prevents 2D tensor from collapsing to 1D when indexing with single element. Fixes convnext_detection model test failure.
|
Hi @cregouby, thanks for the detailed feedback. I also noticed my branch still contains unrelated/generated files and some non-keypoint doc churn, which can make review noisy. I will clean the PR to keep only keypoint-related changes, regenerate docs only for touched exports, and rerun checks before requesting review again. Thanks again for the guidance — I’ll push a cleaned-up revision shortly. |
Chandraveersingh1717
force-pushed
the
keypoints-skeleton
branch
from
0dd2423 to
8479854
Compare
|
2 participants
Completes the GSoC Hard task by finishing draw_keypoints() so keypoints can now be connected with thin lines for clearer pose visualization.
Changes
Added a connectivity parameter to define skeleton edges
Draws lines between keypoints using graphics::segments()
Added a 13-point human pose skeleton example
Fixed a dtype comparison issue in draw_segmentation_masks