Skip to content

Conversation

@ytl0623
Copy link
Contributor

@ytl0623 ytl0623 commented Feb 3, 2026

Fixes #8726

Description

  • Updated monai/transforms/spatial/array.py: Added a Note to RandAffine explaining the inverse relationship for translation and scaling.
  • Updated monai/transforms/spatial/dictionary.py: Added a corresponding Note to RandAffined to ensure consistency for dictionary-based workflows.

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • Breaking change (fix or new feature that would cause existing functionality to change).
  • New tests added to cover the changes.
  • Integration tests passed locally by running ./runtests.sh -f -u --net --coverage.
  • Quick tests passed locally by running ./runtests.sh --quick --unittests --disttests.
  • In-line docstrings updated.
  • Documentation updated, tested make html command in the docs/ folder.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 3, 2026

📝 Walkthrough

Walkthrough

Added explanatory Note blocks about MONAI's backward-mapping (image-to-grid) semantics for translation, scaling, and rotation to the RandAffine docstrings in monai/transforms/spatial/array.py and monai/transforms/spatial/dictionary.py. No public signatures or functional behavior were changed.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding clarification to the backward mapping logic documentation for RandAffine and RandAffined transforms.
Description check ✅ Passed The description covers required sections: it references the fixed issue (#8726), describes changes to both files, marks it as non-breaking, and follows the template structure.
Linked Issues check ✅ Passed The PR adds documentation notes to RandAffine and RandAffined addressing issue #8726's requests for clarification on translation, scaling, and rotation semantics and parameter conventions.
Out of Scope Changes check ✅ Passed All changes are scoped to documentation additions in two transform files addressing issue #8726; no unrelated functional code changes detected.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@monai/transforms/spatial/dictionary.py`:
- Around line 1103-1106: Update the Note in the docstring that references
RandAffine to include rotation semantics: state that, like translation and
scaling, rotation uses backward-mapping so positive rotation values rotate the
sampled image in the opposite direction (i.e., the image is rotated negatively
relative to the transform parameter) and add that the apparent direction (CW vs
CCW) may vary depending on the axis, mirroring RandAffine's wording; modify the
Note near the existing RandAffine reference in
monai/transforms/spatial/dictionary.py to include this rotation sentence.
🧹 Nitpick comments (1)
monai/transforms/spatial/array.py (1)

2439-2445: Good documentation addition addressing backward mapping confusion.

The Note effectively explains the counterintuitive behavior reported in issue #8726. The translation and scaling explanations are accurate for backward mapping semantics.

Optional: Consider clarifying the scaling explanation.

Line 2443 states "Values > 0 decrease the image size" but doesn't reference that these are scale_range input values (before the 1.0 offset mentioned at line 2408 is applied). The current wording is correct but could be clearer for users.

📝 Optional clarification for scale explanation
-            - Scaling: Values > 0 decrease the image size; values in [-1, 0) increase it.
+            - Scaling: scale_range values > 0 (resulting in scale factors > 1.0 after the 1.0 offset) decrease the image size; values in [-1, 0) increase it.

Or more concisely:

-            - Scaling: Values > 0 decrease the image size; values in [-1, 0) increase it.
+            - Scaling: Positive scale_range values decrease the image size; values in [-1, 0) increase it.

Signed-off-by: ytl0623 <david89062388@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Better (corrected?) docs for affine parameters

1 participant