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

[DOC] Improve warnings for BIDS folders #4253

Merged
merged 4 commits into from
Feb 8, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 17 additions & 7 deletions nilearn/glm/first_level/first_level.py
Original file line number Diff line number Diff line change
Expand Up @@ -1268,6 +1268,8 @@ def first_level_from_bids(
derivatives_folder=derivatives_folder,
)

dataset_path = Path(dataset_path).absolute()

kwargs_load_confounds = _check_kwargs_load_confounds(**kwargs)

if drift_model is not None and kwargs_load_confounds is not None:
Expand All @@ -1286,6 +1288,7 @@ def first_level_from_bids(
)

derivatives_path = Path(dataset_path) / derivatives_folder
derivatives_path = derivatives_path.absolute()

# Get metadata for models.
#
Expand Down Expand Up @@ -1325,17 +1328,19 @@ def first_level_from_bids(
t_r = inferred_t_r
if t_r is not None and t_r != inferred_t_r:
warn(
f"'t_r' provided ({t_r}) is different "
f"\n't_r' provided ({t_r}) is different "
f"from the value found in the BIDS dataset ({inferred_t_r}).\n"
"Note this may lead to the wrong model specification."
"Note this may lead to the wrong model specification.",
stacklevel=2,
)
if t_r is not None:
_check_repetition_time(t_r)
else:
warn(
"'t_r' not provided and cannot be inferred from BIDS metadata.\n"
"\n't_r' not provided and cannot be inferred from BIDS metadata.\n"
"It will need to be set manually in the list of models, "
"otherwise their fit will throw an exception."
"otherwise their fit will throw an exception.",
stacklevel=2,
)

# Slice time correction reference time
Expand Down Expand Up @@ -1394,6 +1399,8 @@ def first_level_from_bids(
models_confounds = []

sub_labels = _list_valid_subjects(derivatives_path, sub_labels)
if len(sub_labels) == 0:
raise RuntimeError(f"\nNo subject found in:\n {derivatives_path}")
for sub_label_ in sub_labels:
# Create model
model = FirstLevelModel(
Expand Down Expand Up @@ -1494,8 +1501,10 @@ def _list_valid_subjects(derivatives_path, sub_labels):
sub_labels_exist.append(sub_label_)
else:
warn(
f"Subject label {sub_label_} is not present in the"
" dataset and cannot be processed."
f"\nSubject label '{sub_label_}' is not present "
"in the following dataset and cannot be processed:\n"
f" {derivatives_path}",
stacklevel=3,
)

return set(sub_labels_exist)
Expand Down Expand Up @@ -1933,7 +1942,8 @@ def _make_bids_files_filter(
warn(
f"The filter {filter_} will be skipped. "
f"'{filter_[0]}' is not among the supported filters. "
f"Allowed filters include: {supported_filters}"
f"Allowed filters include: {supported_filters}",
stacklevel=3,
)
continue

Expand Down
25 changes: 18 additions & 7 deletions nilearn/glm/tests/test_first_level.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import itertools
import os
import shutil
import unittest.mock
import warnings
from itertools import product
from pathlib import Path

import numpy as np
Expand Down Expand Up @@ -1461,10 +1463,7 @@ def test_first_level_from_bids_with_subject_labels(bids_dataset):
Check that the incorrect label `foo` raises a warning,
but that we still get a model for existing subject.
"""
warning_message = (
"Subject label foo is not present in"
" the dataset and cannot be processed"
)
warning_message = "Subject label 'foo' is not present in*"
with pytest.warns(UserWarning, match=warning_message):
models, *_ = first_level_from_bids(
dataset_path=bids_dataset,
Expand Down Expand Up @@ -1854,9 +1853,6 @@ def test_missing_trial_type_column_warning(tmp_path_factory):
)


from itertools import product


def test_first_level_from_bids_load_confounds(tmp_path):
"""Test that only a subset of confounds can be loaded."""
n_sub = 2
Expand Down Expand Up @@ -1945,6 +1941,21 @@ def test_first_level_from_bids_load_confounds_warnings(tmp_path):
)


def test_first_level_from_bids_no_subject(tmp_path):
"""Throw error when no subject found."""
bids_path = create_fake_bids_dataset(
base_dir=tmp_path, n_sub=1, n_ses=0, tasks=["main"], n_runs=[2]
)
shutil.rmtree(bids_path / "derivatives" / "sub-01")
with pytest.raises(RuntimeError, match="No subject found in:"):
first_level_from_bids(
dataset_path=bids_path,
task_label="main",
space_label="MNI",
slice_time_ref=None,
)


def test_check_run_tables_errors():
# check high level wrapper keeps behavior
with pytest.raises(ValueError, match="len.* does not match len.*"):
Expand Down
18 changes: 12 additions & 6 deletions nilearn/interfaces/bids/query.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ def _get_metadata_from_bids(
else:
warn(f"'{field}' not found in file {json_files[0]}.", stacklevel=3)
else:
msg_suffix = f" in {bids_path}" if bids_path else ""
warn(f"No bold.json found in BIDS folder{msg_suffix}.")
msg_suffix = f" in:\n {bids_path}" if bids_path else ""
warn(f"\nNo bold.json found in BIDS folder{msg_suffix}.", stacklevel=3)

return None

Expand Down Expand Up @@ -92,8 +92,11 @@ def infer_slice_timing_start_time_from_dataset(bids_path, filters, verbose=0):
)
if not img_specs:
if verbose:
msg_suffix = f" in {bids_path}"
warn(f"No bold.json found in BIDS folder{msg_suffix}.")
msg_suffix = f" in:\n {bids_path}"
warn(
f"\nNo bold.json found in BIDS folder{msg_suffix}.",
stacklevel=3,
)
return None

return _get_metadata_from_bids(
Expand Down Expand Up @@ -137,8 +140,11 @@ def infer_repetition_time_from_dataset(bids_path, filters, verbose=0):

if not img_specs:
if verbose:
msg_suffix = f" in {bids_path}"
warn(f"No bold.json found in BIDS folder{msg_suffix}.")
msg_suffix = f" in:\n {bids_path}"
warn(
f"\nNo bold.json found in BIDS folder{msg_suffix}.",
stacklevel=3,
)
return None

return _get_metadata_from_bids(
Expand Down