Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2537 +/- ##
=======================================
Coverage 88% 88%
=======================================
Files 85 85
Lines 12312 12312
=======================================
+ Hits 10857 10860 +3
+ Misses 1455 1452 -3 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The cookbook lacks required installation and publication metadata and contains misleading claims about universal keypoint ordering.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a self-contained cookbook demonstrating horizontal pose augmentation with COCO-17 left/right keypoint remapping.
Changes:
- Implements deterministic horizontal flipping and semantic row remapping.
- Adds numeric verification and comparative visualization.
- Uses Supervision keypoint annotators for rendering.
Review scores: Code quality 4/5 · Testing 3/5 · Documentation 2/5
File summaries
| File | Description |
|---|---|
docs/notebooks/pose-augmentation-left-right-remapping.ipynb |
Adds the pose-remapping cookbook and visualization. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 6
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+38
to
+41
| "import albumentations as A\n", | ||
| "import cv2\n", | ||
| "import numpy as np\n", | ||
| "import supervision as sv" |
| "id": "48a8a347", | ||
| "metadata": {}, | ||
| "source": [ | ||
| "# Pose augmentation with left/right keypoint remapping\n", |
| "source": [ | ||
| "# Pose augmentation with left/right keypoint remapping\n", | ||
| "\n", | ||
| "`sv.KeyPoints.xy` has a fixed row order: row 1 is always `left_eye`, row 2 is always `right_eye`, and so on for every left/right pair in the skeleton. A horizontal flip mirrors *coordinates*, but on its own it does nothing about *row order* \u2014 after mirroring, the position that used to hold the left eye is now where the right eye visually is, but row 1 is still labeled `left_eye`.\n", |
Comment on lines
+27
to
+36
| "outputs": [ | ||
| { | ||
| "name": "stderr", | ||
| "output_type": "stream", | ||
| "text": [ | ||
| "C:\\Users\\sktam\\AppData\\Local\\Temp\\ccwork\\svenv\\Lib\\site-packages\\tqdm\\auto.py:21: TqdmWarning: IProgress not found. Please update jupyter and ipywidgets. See https://ipywidgets.readthedocs.io/en/stable/user_install.html\n", | ||
| " from .autonotebook import tqdm as notebook_tqdm\n" | ||
| ] | ||
| } | ||
| ], |
| "source": [ | ||
| "## A deterministic pose\n", | ||
| "\n", | ||
| "17 points in COCO order \u2014 the order `sv.KeyPoints.from_ultralytics`, `from_inference`, and RF-DETR's pose models already return. The pose has one arm raised so a wrong flip is visibly different from a correct one, not just numerically." |
| "RIGHT_INDICES = {right for _, right in LEFT_RIGHT_PAIRS}\n", | ||
| "\n", | ||
| "\n", | ||
| "def render(key_points: sv.KeyPoints, canvas: np.ndarray) -> np.ndarray:\n", |
Closes roboflow#2519. sv.KeyPoints.xy has a fixed row order per skeleton (row 1 is always left_eye, row 2 always right_eye, and so on), but a horizontal flip only mirrors coordinates. Albumentations' HorizontalFlip has no notion of which row is semantically left or right, so after mirroring, row 1 still says left_eye even though its position is now where the right eye visually is. The cookbook builds a small deterministic pose (right arm raised, no model or dataset needed), flips it, and shows the fix: after mirroring coordinates, also swap each left/right pair's row so the label matches the mirrored side. AlbumentationsX's KeypointParams(label_mapping=...) automates this same swap inside the transform for larger pipelines; doing it explicitly here (rather than adding a hard torch dependency just to demonstrate one kwarg) shows exactly what that option does under the hood, and keeps the cookbook as self-contained as the issue asked for. Includes a numeric verification (every remapped row lands exactly where albumentations' own pixel-index mirror, width - 1 - x, says it should) and a rendered comparison (left joints blue, right red) that makes the bug visible: the naive flip still colors the raised arm red after mirroring, the corrected version colors it blue. Executed end-to-end via jupyter nbconvert before committing; matches the repo's existing notebooks in shipping baked-in outputs (mkdocs-jupyter's execute: false renders them as-is).
- add the missing install cell (supervision, albumentations, opencv-python) and open in colab badge, per CONTRIBUTING.md's cookbook checklist - add the cookbooks.html card so it's discoverable from the cookbooks page - scope the "row 1 is always left_eye" and "these converters return coco-17" claims to coco-17 specifically, since keypoint order actually comes from the source model/skeleton - clear the committed stderr output that leaked a local windows path and an unrelated tqdm warning (fixed at the source by installing ipywidgets, not just stripped from the diff) - add a docstring to the render() helper per agents.md - re-executed end to end so every output is real, not hand-edited
adhavan18
force-pushed
the
feat/pose-flip-cookbook
branch
from
September 9, 2026 15:59
c4c7c4a to
6b7f782
Compare
Contributor
Author
|
addressed all 6 points from the copilot review:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Before submitting
Description
A self-contained cookbook showing how to augment
sv.KeyPointswith a horizontal flip while preserving left/right landmark semantics, as requested.Type of Change
Motivation and Context
Closes #2519.
sv.KeyPoints.xyhas a fixed row order per skeleton: for a COCO-17 pose, row 1 is alwaysleft_eye, row 2 alwaysright_eye, and so on. A horizontal flip mirrors coordinates, but Albumentations'HorizontalFliphas no notion of which row is semantically left or right, so after mirroring, row 1 still saysleft_eyeeven though its position is now where the right eye visually is. The cookbook makes this concrete and shows the fix.Changes Made
docs/notebooks/pose-augmentation-left-right-remapping.ipynbA.HorizontalFlip, then fixes the row order by swapping each left/right pair after mirroringwidth - 1 - x)Testing
jupyter nbconvert --executebefore committing; zero errors, all assertions passScreenshots/Videos (optional)
The notebook's own rendered output (visible in the Files changed diff): three panels — original pose, naive flip (still labels the raised arm "right" after mirroring), corrected flip (correctly relabels it "left").
Additional Notes
The issue mentions AlbumentationsX's
KeypointParams(label_mapping=...), which automates this exact row swap inside the transform. I deliberately didn't use it here: AlbumentationsX requirestorchunconditionally (even for a plainHorizontalFlip), which is a multi-GB install just to demonstrate one kwarg, and works against the issue's own stated goal of a cookbook that "needs no model or dataset download." Doing the remap explicitly with vanillaalbumentationskeeps the notebook self-contained and, I'd argue, is more instructive for a cookbook — it shows exactly what that option automates rather than hiding it behind a library flag. Happy to add an AlbumentationsX variant alongside this if that's preferred.