Skip to content

fix(misc): correct docs wording and visualiser axis-count assertions#88

Draft
Copilot wants to merge 3 commits into
mainfrom
copilot/fix-missing-space-correct-verb-form
Draft

fix(misc): correct docs wording and visualiser axis-count assertions#88
Copilot wants to merge 3 commits into
mainfrom
copilot/fix-missing-space-correct-verb-form

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 18, 2026

Summary

Applies six requested copy/test fixes: three documentation grammar/spacing corrections and three test assertion tightenings in Captum image visualiser tests. The test updates now validate exact subplot layout expectations instead of permissive lower bounds.

  • Docs wording fixes

    • docs/contributor/releases.md: add missing space before toctree after `going-stable-1-0`.
    • docs/index.md: change “then ran” → “then run”.
    • docs/using-raitap/installation.md: change “was currently tested” → “is currently tested”.
  • Transparency visualiser test expectations

    • src/raitap/transparency/visualisers/tests/test_visualisers.py
      • test_visualise_tensor: len(fig.axes) == 8 (4 samples × 2 panels).
      • test_max_samples_limit: len(fig.axes) == 8 for max_samples=4.
      • test_can_disable_original_image_panel: len(fig.axes) == 2 when include_original_image=False and max_samples=2.
fig = visualiser.visualise(attributions)
assert len(fig.axes) == 8  # 2 axes per sample (original + attribution) for 4 samples

Checklist

  • CI — Required checks are green
  • Breaking changes — If this PR breaks compatibility (API, configs, file formats, etc.), the title uses ! after the type or scope (e.g. feat!: or feat(transparency)!:). I understand that this change will have a direct, disruptive impact on package users. I ensured that this change is absolutely necessary and cannot be delayed.
  • Contributor guide — I’ve read Pull requests and commit messages before requesting review.

Optional

  • Issue — A GitHub issues is linked to this PR ("Development" section in the right sidebar).
  • Docs — User or contributor docs updated where needed.
  • Tests — New or updated tests cover the change.
Original prompt
Please apply the following diffs and create a pull request.
Once the PR is ready, give it a title based on the messages of the fixes being applied.

[{"message":"Missing space between 'going-stable-1-0' and 'toctree'. Should be `going-stable-1-0` toctree.","fixFiles":[{"filePath":"docs/contributor/releases.md","diff":"diff --git a/docs/contributor/releases.md b/docs/contributor/releases.md\n--- a/docs/contributor/releases.md\n+++ b/docs/contributor/releases.md\n@@ -39,7 +39,7 @@\n \n ## 3. Update the contributor guide\n \n-Delete this file and update {doc}`./index` to remove the `going-stable-1-0`toctree entry.\n+Delete this file and update {doc}`./index` to remove the `going-stable-1-0` toctree entry.\n \n ## 4. Cutting 1.0.0\n \n"}]},{"message":"Corrected verb form from 'ran' to 'run'.","fixFiles":[{"filePath":"docs/index.md","diff":"diff --git a/docs/index.md b/docs/index.md\n--- a/docs/index.md\n+++ b/docs/index.md\n@@ -13,7 +13,7 @@\n \n ## Where does it fit in my workflow?\n \n-RAITAP is configured via YAML [Hydra](https://hydra.cc/) configs or CLI flags, and then ran via a CLI command.\n+RAITAP is configured via YAML [Hydra](https://hydra.cc/) configs or CLI flags, and then run via a CLI command.\n \n This means it can be used either as:\n \n"}]},{"message":"The phrase 'was currently tested' is grammatically inconsistent. Use either 'was tested' (past tense) or 'is currently tested' (present tense).","fixFiles":[{"filePath":"docs/using-raitap/installation.md","diff":"diff --git a/docs/using-raitap/installation.md b/docs/using-raitap/installation.md\n--- a/docs/using-raitap/installation.md\n+++ b/docs/using-raitap/installation.md\n@@ -15,7 +15,7 @@\n ```\n \n :::{note}\n-RAITAP was currently tested with Python 3.13.x. Ensure your project matches this requirement, or expect possible issues.\n+RAITAP is currently tested with Python 3.13.x. Ensure your project matches this requirement, or expect possible issues.\n :::\n \n ## 2. Install optional dependencies\n"}]},{"message":"The comment says 'at least one axes per sample', but the assertion checks for >= 4 axes when sample_images has 4 samples. This doesn't account for the case where include_original_image=True (default), which would create 2 axes per sample (original + attribution). The test should verify 8 axes (2 per sample × 4 samples) to properly validate the paired layout, or explicitly set include_original_image=False if testing single-panel output.","fixFiles":[{"filePath":"src/raitap/transparency/visualisers/tests/test_visualisers.py","diff":"diff --git a/src/raitap/transparency/visualisers/tests/test_visualisers.py b/src/raitap/transparency/visualisers/tests/test_visualisers.py\n--- a/src/raitap/transparency/visualisers/tests/test_visualisers.py\n+++ b/src/raitap/transparency/visualisers/tests/test_visualisers.py\n@@ -40,7 +40,7 @@\n \n         fig = visualiser.visualise(attributions)\n         assert fig is not None\n-        assert len(fig.axes) >= 4  # at least one axes per sample\n+        assert len(fig.axes) == 8  # 2 axes per sample (original + attribution) for 4 samples\n \n     @pytest.mark.usefixtures(\"needs_captum\")\n     def test_max_samples_limit(self) -> None:\n"}]},{"message":"Similar to Comment 1, this assertion checks for >= 4 axes when max_samples=4, but doesn't validate the expected paired layout. With default include_original_image=True and max_samples=4, the figure should have 8 axes total. The test should verify the exact count to ensure proper visualization layout.","fixFiles":[{"filePath":"src/raitap/transparency/visualisers/tests/test_visualisers.py","diff":"diff --git a/src/raitap/transparency/visualisers/tests/test_visualisers.py b/src/raitap/transparency/visualisers/tests/test_visualisers.py\n--- a/src/raitap/transparency/visualisers/tests/test_visualisers.py\n+++ b/src/raitap/transparency/visualisers/tests/test_visualisers.py\n@@ -48,7 +48,7 @@\n         large_batch = torch.randn(64, 3, 32, 32)\n \n         fig = visualiser.visualise(large_batch, max_samples=4)\n-        assert len(fig.axes) >= 4\n+        assert len(fig.axes) == 8\n \n     @pytest.mark.usefixtures(\"needs_captum\")\n     def test_overlay_with_inputs(self, sample_images: torch.Tensor) -> None:\n"}]},{"message":"When include_original_image=False and max_samples=2, this should produce exactly 2 axes (one attribution panel per sample), not >= 2. The test should assert the exact count to verify that the original image panels are indeed disabled.","fixFiles":[{"filePath":"src/raitap/transparency/visualisers/tests/test_visualisers.py","diff":"diff --git a/src/raitap/transparency/visualisers/tests/test_visualisers.py b/src/raitap/transparency/visualisers/tests/test_visualisers.py\n--- a/src/raitap/transparency/visualisers/tests/test_visualisers.py\n+++ b/src/raitap/transparency/visualisers/tests/test_visualisers.py\n@@ -81,7 +81,7 @@\n \n         fig = visualiser.visualise(attributions, inputs=sample_images, max_samples=2)\n \n-        a...

Agent-Logs-Url: https://github.com/CAIIVS/raitap/sessions/19498a80-3be8-4987-9d47-4c9c045ac319

Co-authored-by: stanlrt <63193889+stanlrt@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix missing space in releases.md and correct verb form in index.md fix(misc): correct docs wording and visualiser axis-count assertions Apr 18, 2026
Copilot AI requested a review from stanlrt April 18, 2026 12:10
@stanlrt stanlrt marked this pull request as ready for review April 18, 2026 16:58
Copilot AI review requested due to automatic review settings April 18, 2026 16:58
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR applies small documentation wording fixes and tightens Captum image visualiser tests to assert exact Matplotlib subplot/axes layout expectations rather than permissive lower bounds.

Changes:

  • Fix minor grammar/spacing issues across three documentation pages.
  • Tighten Captum image visualiser tests by asserting specific fig.axes counts for selected scenarios.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.

File Description
docs/contributor/releases.md Fixes missing space in contributor release instructions.
docs/index.md Corrects wording (“ran” → “run”) in docs homepage.
docs/using-raitap/installation.md Updates Python version testing sentence to consistent present tense.
src/raitap/transparency/visualisers/tests/test_visualisers.py Tightens Captum image visualiser axes-count assertions.

fig = visualiser.visualise(attributions)
assert fig is not None
assert len(fig.axes) >= 4 # at least one axes per sample
assert len(fig.axes) == 8 # 2 axes per sample (original + attribution) for 4 samples
Copy link

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

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

The inline comment claims the 8 axes are from “original + attribution”, but this test calls visualise(attributions) without inputs, so the original-image panel cannot be rendered. Either pass inputs=sample_images (and update the expected axes count accordingly), or update the comment/expectation to reflect what actually creates the extra axes here (typically Captum’s per-sample colorbar axes when show_colorbar=True).

Suggested change
assert len(fig.axes) == 8 # 2 axes per sample (original + attribution) for 4 samples
assert len(fig.axes) == 8 # 2 axes per sample for 4 samples: attribution plot + colorbar

Copilot uses AI. Check for mistakes.
Comment on lines 82 to 85
fig = visualiser.visualise(attributions, inputs=sample_images, max_samples=2)

assert len(fig.axes) >= 2
assert len(fig.axes) == 2
assert fig.axes[0].get_title() == ""
Copy link

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

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

CaptumImageVisualiser defaults to show_colorbar=True, and in the non-paired layout Captum typically adds an extra colorbar axis per sample. With max_samples=2 this usually yields 4 axes (2 plots + 2 colorbars), so len(fig.axes) == 2 is likely incorrect/flaky. To assert exactly 2 axes, set show_colorbar=False for this test, or update the expected axes count to include the colorbar axes.

Copilot uses AI. Check for mistakes.
@stanlrt stanlrt marked this pull request as draft May 8, 2026 12:55
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.

3 participants