Skip to content

Commit

Permalink
Do not zero-pad indices in BIDSPath entities anymore; do not warn abo…
Browse files Browse the repository at this point in the history
…ut prepending missing dots for extensions (#1215)

* [BUG] Allow int subject etc

* fix

* remove magic padding

* fix tests

* change position of logic back

* fix logic

* revert some changes

* update changelog

* remove zfill again

* bring back test

---------

Co-authored-by: Stefan Appelhoff <[email protected]>
  • Loading branch information
alexrockhill and sappelhoff authored Feb 22, 2024
1 parent eabe513 commit e41b517
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 35 deletions.
8 changes: 7 additions & 1 deletion doc/whats_new.rst
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ The following authors contributed for the first time. Thank you so much! 🤩

The following authors had contributed before. Thank you for sticking around! 🤘

* `Alex Rockhill`_
* `Eric Larson`_
* `Laetitia Fesselier`_
* `Richard Höchenberger`_
* `Stefan Appelhoff`_
Expand All @@ -38,6 +40,10 @@ Detailed list of changes

- The experimental support for running MNE-BIDS examples from your browser using Binder has
been removed, by `Stefan Appelhoff`_ (:gh:`1202`)
- MNE-BIDS will no longer zero-pad ("zfill") entity indices passed to :class:`~mne_bids.BIDSPath`.
For example, If ``run=1`` is passed to MNE-BIDS, it will no longer be silently auto-converted to ``run-01``, by `Alex Rockhill`_ (:gh:`1215`)
- MNE-BIDS will no longer warn about missing leading punctuation marks for extensions passed :class:`~mne_bids.BIDSPath`.
For example, MNE-BIDS will now silently auto-convert ``edf`` to ```.edf``, by `Alex Rockhill`_ (:gh:`1215`)

🛠 Requirements
^^^^^^^^^^^^^^^
Expand All @@ -52,7 +58,7 @@ Detailed list of changes
^^^^^^^^^^^^

- The datatype in the dataframe returned by :func:`mne_bids.stats.count_events` is now
``pandas.Int64Dtype`` instead of ``float64``.
``pandas.Int64Dtype`` instead of ``float64``, by `Eric Larson`_ (:gh:`1227`)

⚕️ Code health
^^^^^^^^^^^^^^
Expand Down
14 changes: 1 addition & 13 deletions mne_bids/path.py
Original file line number Diff line number Diff line change
Expand Up @@ -966,24 +966,12 @@ def update(self, *, check=None, **kwargs):
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}"
kwargs[key] = f"{val}"

# 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}'
# )
del extension

# error check entities
Expand Down
36 changes: 15 additions & 21 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 @@ -1474,12 +1474,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

0 comments on commit e41b517

Please sign in to comment.