Follow-up for Fixing SSIM Test Method Names

[benediktjohannes]

This fixes some other issues which were described in #8746 .

Description

This is a follow-up for #8746, thanks and credits to @ericspod! 👍

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]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 4aceb794-8a5f-4a6e-8a61-638736f9ce97

📥 Commits

Reviewing files that changed from the base of the PR and between bee416e and 6aef0fd.

📒 Files selected for processing (1)

• tests/metrics/test_ssim_metric.py

🚧 Files skipped from review as they are similar to previous changes (1)

• tests/metrics/test_ssim_metric.py

---

📝 Walkthrough

Walkthrough

Three test methods in tests/metrics/test_ssim_metric.py were renamed to snake_case: test2d_gaussian → test_2d_gaussian, test2d_uniform → test_2d_uniform, and test3d_gaussian → test_3d_gaussian. Additionally, two assertions were made sign-agnostic by wrapping the difference in abs(...) < 0.000001 for the 2D gaussian and 2D uniform tests; the 3D gaussian assertion remains unchanged.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	Title accurately reflects the main change—renaming test methods to follow Python naming conventions.
Description check	✅ Passed	Description references issue #8746, credits contributors, marks as non-breaking, and matches template structure with appropriate checkboxes.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

tests/metrics/test_ssim_metric.py (1)

63-74: ⚠️ Potential issue | 🟠 Major

input_ill_input_shape and mismatch_y_pred_and_y are never executed.

Both methods lack the test_ prefix, so the test runner silently ignores them. Given this PR is specifically fixing test method names, these should be fixed here too.

🐛 Proposed fix

-    def input_ill_input_shape(self):
+    def test_input_ill_input_shape(self):
         with self.assertRaises(ValueError):
             metric = SSIMMetric(spatial_dims=3)
             metric(torch.randn(1, 1, 16, 16), torch.randn(1, 1, 16, 16))

         with self.assertRaises(ValueError):
             metric = SSIMMetric(spatial_dims=2)
             metric(torch.randn(1, 1, 16, 16, 16), torch.randn(1, 1, 16, 16, 16))

-    def mismatch_y_pred_and_y(self):
+    def test_mismatch_y_pred_and_y(self):
         with self.assertRaises(ValueError):
             compute_ssim_and_cs(y_pred=torch.randn(1, 1, 16, 8), y=torch.randn(1, 1, 16, 16), spatial_dims=2)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/metrics/test_ssim_metric.py` around lines 63 - 74, Rename the two test
methods so the test runner executes them: change input_ill_input_shape to
test_input_ill_input_shape and mismatch_y_pred_and_y to
test_mismatch_y_pred_and_y; these methods reference SSIMMetric and
compute_ssim_and_cs, so update only the method names (no other logic) to ensure
the assertions raising ValueError run during test collection.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/metrics/test_ssim_metric.py`:
- Line 35: The assertion uses a one-sided check (expected_value - result.item()
< 0.000001) which allows results greater than expected_value to pass; replace it
with a symmetric tolerance check by using abs(expected_value - result.item()) <
1e-6 or unittest's self.assertAlmostEqual(result.item(), expected_value,
places=6) for the assertions that reference expected_value and result.item()
(apply the same change to the other two occurrences mentioned).

---

Outside diff comments:
In `@tests/metrics/test_ssim_metric.py`:
- Around line 63-74: Rename the two test methods so the test runner executes
them: change input_ill_input_shape to test_input_ill_input_shape and
mismatch_y_pred_and_y to test_mismatch_y_pred_and_y; these methods reference
SSIMMetric and compute_ssim_and_cs, so update only the method names (no other
logic) to ensure the assertions raising ValueError run during test collection.

[ericspod]

Hi @benediktjohannes, please look at what coderabbit as said here, the other method names also should be changed as they aren't getting run as tests it seems and line 35 should be self.assertTrue(abs(expected_value - result.item()) < 1e-6) . Thanks!