Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for IBMA estimators with t-statistic images in Reports #896

Merged
merged 4 commits into from
Aug 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions nimare/reports/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
"method": "Method",
"alpha": "Alpha",
"prior": "Prior",
"tau2": "Between-study variance",
"use_sample_size": "Use sample size for weights",
"normalize_contrast_weights": "Normalize by the number of contrasts",
"two_sided": "Two-sided test",
Expand Down Expand Up @@ -483,10 +484,15 @@
)
elif meta_type == "IBMA":
# Use "z_maps", for Fishers, and Stouffers; otherwise use "beta_maps".
key_maps = "z_maps" if "z_maps" in self.results.estimator.inputs_ else "beta_maps"
INPUT_TYPE_LABELS = {"z_maps": "Z", "t_maps": "T", "beta_maps": "Beta"}
for key_maps, x_label in INPUT_TYPE_LABELS.items():
if key_maps in self.results.estimator.inputs_:
break
else:
key_maps, x_label = "beta_maps", "Beta"

Check warning on line 492 in nimare/reports/base.py

View check run for this annotation

Codecov / codecov/patch

nimare/reports/base.py#L492

Added line #L492 was not covered by tests

maps_arr = self.results.estimator.inputs_[key_maps]
ids_ = self.results.estimator.inputs_["id"]
x_label = "Z" if key_maps == "z_maps" else "Beta"

if self.results.estimator.aggressive_mask:
_plot_relcov_map(
Expand Down Expand Up @@ -526,12 +532,12 @@
if self.results.estimator.aggressive_mask:
voxel_mask = self.results.estimator.inputs_["aggressive_mask"]
corr = np.corrcoef(
self.results.estimator.inputs_["z_maps"][:, voxel_mask],
self.results.estimator.inputs_[key_maps][:, voxel_mask],
rowvar=True,
Comment on lines 532 to 536
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: Add a comment explaining the different approaches for aggressive and non-aggressive masking

The reasoning behind using different methods for correlation calculation based on the masking approach is not immediately clear. A brief comment would improve code understanding.

Suggested change
if self.results.estimator.aggressive_mask:
voxel_mask = self.results.estimator.inputs_["aggressive_mask"]
corr = np.corrcoef(
self.results.estimator.inputs_["z_maps"][:, voxel_mask],
self.results.estimator.inputs_[key_maps][:, voxel_mask],
rowvar=True,
# Calculate correlation based on masking approach
if self.results.estimator.aggressive_mask:
voxel_mask = self.results.estimator.inputs_["aggressive_mask"]
corr = np.corrcoef(
self.results.estimator.inputs_[key_maps][:, voxel_mask],
rowvar=True,
)
else:
corr = np.corrcoef(self.results.estimator.inputs_[key_maps], rowvar=True)

)
else:
corr = np.zeros((n_studies, n_studies), dtype=float)
for bag in self.results.estimator.inputs_["data_bags"]["z_maps"]:
for bag in self.results.estimator.inputs_["data_bags"][key_maps]:
study_bag = bag["study_mask"]
corr[np.ix_(study_bag, study_bag)] = np.corrcoef(
bag["values"],
Expand Down
27 changes: 23 additions & 4 deletions nimare/tests/test_reports.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from nimare.correct import FWECorrector
from nimare.diagnostics import FocusCounter, Jackknife
from nimare.meta.cbma import ALESubtraction
from nimare.meta.ibma import Stouffers
from nimare.meta.ibma import FixedEffectsHedges, Stouffers
from nimare.reports.base import run_reports
from nimare.workflows import CBMAWorkflow, IBMAWorkflow, PairwiseCBMAWorkflow

Expand Down Expand Up @@ -75,17 +75,36 @@ def test_reports_ibma_smoke(tmp_path_factory, testdata_ibma, aggressive_mask):
"""Smoke test for IBMA reports."""
tmpdir = tmp_path_factory.mktemp("test_reports_ibma_smoke")

# Generate a report with z maps as inputs
stouffers_dir = op.join(tmpdir, "stouffers")
workflow = IBMAWorkflow(
estimator=Stouffers(aggressive_mask=aggressive_mask),
corrector="fdr",
diagnostics="jackknife",
voxel_thresh=3.2,
output_dir=tmpdir,
output_dir=stouffers_dir,
)
results = workflow.fit(testdata_ibma)

run_reports(results, tmpdir)
run_reports(results, stouffers_dir)

filename = "report.html"
outpath = op.join(tmpdir, filename)
outpath = op.join(stouffers_dir, filename)
assert op.isfile(outpath)
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion (testing): Consider adding edge case tests for invalid or missing t-statistic images

While the current tests cover the basic functionality, it would be beneficial to add tests for edge cases such as invalid t-statistic images or missing images. This will ensure the robustness of the new feature.

def test_invalid_tstat_images():
    with pytest.raises(ValueError):
        IBMAWorkflow(invalid_image_path)

def test_missing_tstat_images():
    with pytest.raises(FileNotFoundError):
        IBMAWorkflow(non_existent_image_path)


# Generate a report with t maps as inputs
hedges_dir = op.join(tmpdir, "hedges")
workflow = IBMAWorkflow(
estimator=FixedEffectsHedges(aggressive_mask=aggressive_mask),
corrector="fdr",
diagnostics="jackknife",
voxel_thresh=3.2,
output_dir=hedges_dir,
)
results = workflow.fit(testdata_ibma)

run_reports(results, hedges_dir)

filename = "report.html"
outpath = op.join(hedges_dir, filename)
assert op.isfile(outpath)
Loading