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

Do not zero-pad indices in BIDSPath entities anymore; do not warn about prepending missing dots for extensions #1215

Merged
merged 14 commits into from
Feb 22, 2024
2 changes: 1 addition & 1 deletion doc/whats_new.rst
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ Detailed list of changes
🪲 Bug fixes
^^^^^^^^^^^^

- nothing yet
- Allow integer types to be passed to :class:`mne_bids.BIDSPath` by `Alex Rockhill`_ (:gh:`1215`)

⚕️ Code health
^^^^^^^^^^^^^^
Expand Down
17 changes: 3 additions & 14 deletions mne_bids/path.py
Original file line number Diff line number Diff line change
Expand Up @@ -958,32 +958,21 @@ def update(self, *, check=None, **kwargs):
f"Key must be one of " f"{ALLOWED_PATH_ENTITIES}, got {key}"
)

if isinstance(val, int):
kwargs[key] = f"{val}"

if ENTITY_VALUE_TYPE[key] == "label":
_validate_type(val, types=(None, str), item_name=key)
else:
assert ENTITY_VALUE_TYPE[key] == "index"
_validate_type(val, types=(int, str, None), item_name=key)
if isinstance(val, str) and not val.isdigit():
raise ValueError(f"{key} is not an index (Got {val})")
elif isinstance(val, int):
kwargs[key] = f"{val:02}"

# ensure extension starts with a '.'
extension = kwargs.get("extension")
if extension is not None and not extension.startswith("."):
warn(
f'extension should start with a period ".", but got: '
f'"{extension}". Prepending "." to form: ".{extension}". '
f"This will raise an exception starting with MNE-BIDS 0.12.",
category=FutureWarning,
)
kwargs["extension"] = f".{extension}"
# Uncomment in 0.12, and remove above code:
#
# raise ValueError(
# f'Extension must start wie a period ".", but got: '
# f'{extension}'
# )
sappelhoff marked this conversation as resolved.
Show resolved Hide resolved
del extension

# error check entities
Expand Down
39 changes: 15 additions & 24 deletions mne_bids/tests/test_path.py
Original file line number Diff line number Diff line change
Expand Up @@ -352,10 +352,10 @@ def test_parse_ext():
@pytest.mark.parametrize(
"fname",
[
"sub-01_ses-02_task-test_run-3_split-01_meg.fif",
"sub-01_ses-02_task-test_run-3_split-01",
"/bids_root/sub-01/ses-02/meg/sub-01_ses-02_task-test_run-3_split-01_meg.fif", # noqa: E501
"sub-01/ses-02/meg/sub-01_ses-02_task-test_run-3_split-01_meg.fif",
"sub-01_ses-02_task-test_run-3_split-1_meg.fif",
"sub-01_ses-02_task-test_run-3_split-1",
"/bids_root/sub-01/ses-02/meg/sub-01_ses-02_task-test_run-3_split-1_meg.fif", # noqa: E501
"sub-01/ses-02/meg/sub-01_ses-02_task-test_run-3_split-1_meg.fif",
],
)
def test_get_bids_path_from_fname(fname):
Expand All @@ -377,12 +377,12 @@ def test_get_bids_path_from_fname(fname):
@pytest.mark.parametrize(
"fname",
[
"sub-01_ses-02_task-test_run-3_split-01_desc-filtered_meg.fif",
"sub-01_ses-02_task-test_run-3_split-01_desc-filtered.fif",
"sub-01_ses-02_task-test_run-3_split-01_desc-filtered",
"sub-01_ses-02_task-test_run-3_split-1_desc-filtered_meg.fif",
"sub-01_ses-02_task-test_run-3_split-1_desc-filtered.fif",
"sub-01_ses-02_task-test_run-3_split-1_desc-filtered",
(
"/bids_root/sub-01/ses-02/meg/"
+ "sub-01_ses-02_task-test_run-3_split-01_desc-filtered_meg.fif"
+ "sub-01_ses-02_task-test_run-3_split-1_desc-filtered_meg.fif"
),
],
)
Expand All @@ -394,7 +394,7 @@ def test_get_entities_from_fname(fname):
assert params["run"] == "3"
assert params["task"] == "test"
assert params["description"] == "filtered"
assert params["split"] == "01"
assert params["split"] == "1"
assert list(params.keys()) == [
"subject",
"session",
Expand Down Expand Up @@ -471,7 +471,7 @@ def test_get_entities_from_fname_errors(fname):
# First candidate is disqualified (session doesn't match)
(["sub-01_ses-01", "sub-01_ses-02"], ["sub-01_ses-02"]),
# Multiple equally good candidates
(["sub-01_run-01", "sub-01_run-02"], ["sub-01_run-01", "sub-01_run-02"]),
(["sub-01_run-1", "sub-01_run-2"], ["sub-01_run-1", "sub-01_run-2"]),
],
)
def test_find_best_candidates(candidate_list, best_candidates):
Expand Down Expand Up @@ -796,7 +796,7 @@ def test_bids_path(return_bids_test_dir):

# test that split gets properly set
bids_path.update(split=1)
assert bids_path.basename == "sub-01_ses-02_task-03_split-01_ieeg.mat"
assert bids_path.basename == "sub-01_ses-02_task-03_split-1_ieeg.mat"

# test home dir expansion
bids_path = BIDSPath(root="~/foo")
Expand Down Expand Up @@ -858,7 +858,7 @@ def test_make_filenames():
datatype="ieeg",
)
expected_str = (
"sub-one_ses-two_task-three_acq-four_run-01_proc-six_" "rec-seven_ieeg.json"
"sub-one_ses-two_task-three_acq-four_run-1_proc-six_" "rec-seven_ieeg.json"
)
assert BIDSPath(**prefix_data).basename == expected_str
assert (
Expand All @@ -869,7 +869,7 @@ def test_make_filenames():
# subsets of keys works
assert (
BIDSPath(subject="one", task="three", run=4).basename
== "sub-one_task-three_run-04"
== "sub-one_task-three_run-4"
)
assert (
BIDSPath(subject="one", task="three", suffix="meg", extension=".json").basename
Expand Down Expand Up @@ -897,7 +897,7 @@ def test_make_filenames():
basename = BIDSPath(**prefix_data, check=False)
assert (
basename.basename
== "sub-one_ses-two_task-three_acq-four_run-01_proc-six_rec-seven_ieeg.h5"
== "sub-one_ses-two_task-three_acq-four_run-1_proc-six_rec-seven_ieeg.h5"
) # noqa

# what happens with scans.tsv file
Expand Down Expand Up @@ -960,7 +960,7 @@ def test_match(return_bids_test_dir):
bids_path_01 = BIDSPath(root=bids_root, run="01")
paths = bids_path_01.match()
assert len(paths) == 3
assert paths[0].basename == ("sub-01_ses-01_task-testing_run-01_" "channels.tsv")
assert paths[0].basename == ("sub-01_ses-01_task-testing_run-01_channels.tsv")

bids_path_01 = BIDSPath(root=bids_root, subject="unknown")
paths = bids_path_01.match()
Expand Down Expand Up @@ -1342,9 +1342,6 @@ def test_find_emptyroom_no_meas_date(tmp_path):

def test_bids_path_label_vs_index_entity():
"""Test entities that must be strings vs those that may be an int."""
match = "subject must be an instance of None or str"
with pytest.raises(TypeError, match=match):
BIDSPath(subject=1)
match = "root must be an instance of path-like or None"
with pytest.raises(TypeError, match=match):
BIDSPath(root=1, subject="01")
Expand Down Expand Up @@ -1474,12 +1471,6 @@ def test_setting_entities():
assert getattr(bids_path, entity_name) is None


def test_deprecation():
"""Test deprecated behavior."""
with pytest.warns(FutureWarning, match="This will raise an exception"):
BIDSPath(extension="vhdr") # no leading period


def test_dont_create_dirs_on_fpath_access(tmp_path):
"""Regression test: don't create directories when accessing .fpath."""
bp = BIDSPath(subject="01", datatype="eeg", root=tmp_path)
Expand Down