From 2a922b2529ed65985da82ce95e6c54ef76ee4bcb Mon Sep 17 00:00:00 2001 From: Alex Rockhill Date: Wed, 10 Jan 2024 12:00:40 -0800 Subject: [PATCH 01/10] [BUG] Allow int subject etc --- mne_bids/path.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mne_bids/path.py b/mne_bids/path.py index d05f01b47..cb72f02dd 100644 --- a/mne_bids/path.py +++ b/mne_bids/path.py @@ -959,6 +959,8 @@ def update(self, *, check=None, **kwargs): ) if ENTITY_VALUE_TYPE[key] == "label": + if isinstance(val, int): + val = str(val) _validate_type(val, types=(None, str), item_name=key) else: assert ENTITY_VALUE_TYPE[key] == "index" From fbd83e448f953c2987519ff7d0c3713d7d262940 Mon Sep 17 00:00:00 2001 From: Alex Rockhill Date: Thu, 11 Jan 2024 09:12:40 -0800 Subject: [PATCH 02/10] fix --- doc/whats_new.rst | 2 +- mne_bids/path.py | 19 +++---------------- mne_bids/tests/test_path.py | 9 --------- 3 files changed, 4 insertions(+), 26 deletions(-) diff --git a/doc/whats_new.rst b/doc/whats_new.rst index 0ab55702b..8269e27a3 100644 --- a/doc/whats_new.rst +++ b/doc/whats_new.rst @@ -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 ^^^^^^^^^^^^^^ diff --git a/mne_bids/path.py b/mne_bids/path.py index cb72f02dd..5da89c57b 100644 --- a/mne_bids/path.py +++ b/mne_bids/path.py @@ -958,34 +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:02}" + if ENTITY_VALUE_TYPE[key] == "label": - if isinstance(val, int): - val = str(val) _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}' - # ) del extension # error check entities diff --git a/mne_bids/tests/test_path.py b/mne_bids/tests/test_path.py index dedf69d68..76d89dca1 100644 --- a/mne_bids/tests/test_path.py +++ b/mne_bids/tests/test_path.py @@ -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") @@ -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) From 2fcaf7934b31ef0c1e0d0dfba666a996ca2214e5 Mon Sep 17 00:00:00 2001 From: Alex Rockhill Date: Fri, 12 Jan 2024 08:12:48 -0800 Subject: [PATCH 03/10] remove magic padding --- mne_bids/path.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mne_bids/path.py b/mne_bids/path.py index 5da89c57b..5008b7dcc 100644 --- a/mne_bids/path.py +++ b/mne_bids/path.py @@ -959,7 +959,7 @@ def update(self, *, check=None, **kwargs): ) if isinstance(val, int): - kwargs[key] = f"{val:02}" + kwargs[key] = f"{val}" if ENTITY_VALUE_TYPE[key] == "label": _validate_type(val, types=(None, str), item_name=key) From e3401674aa6c8eecc5256737a802093406c1427e Mon Sep 17 00:00:00 2001 From: Alex Rockhill Date: Fri, 12 Jan 2024 08:20:21 -0800 Subject: [PATCH 04/10] fix tests --- mne_bids/tests/test_path.py | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/mne_bids/tests/test_path.py b/mne_bids/tests/test_path.py index 76d89dca1..7134fd9a7 100644 --- a/mne_bids/tests/test_path.py +++ b/mne_bids/tests/test_path.py @@ -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): @@ -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" ), ], ) @@ -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", @@ -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): @@ -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") @@ -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 ( @@ -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 @@ -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 @@ -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() From e35e709fa38095bc3d80f3ed7a52ea96540f8f26 Mon Sep 17 00:00:00 2001 From: Stefan Appelhoff Date: Thu, 22 Feb 2024 10:51:43 +0100 Subject: [PATCH 05/10] change position of logic back --- mne_bids/path.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/mne_bids/path.py b/mne_bids/path.py index 5008b7dcc..29f8a46bb 100644 --- a/mne_bids/path.py +++ b/mne_bids/path.py @@ -958,9 +958,6 @@ 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: @@ -968,6 +965,8 @@ def update(self, *, check=None, **kwargs): _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})") + else: + kwargs[key] = f"{val}" # ensure extension starts with a '.' extension = kwargs.get("extension") From 7ecab083262b26c1320d858e84ee975c38cfe289 Mon Sep 17 00:00:00 2001 From: Stefan Appelhoff Date: Thu, 22 Feb 2024 10:52:39 +0100 Subject: [PATCH 06/10] fix logic --- mne_bids/path.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mne_bids/path.py b/mne_bids/path.py index 29f8a46bb..39e6d5bb1 100644 --- a/mne_bids/path.py +++ b/mne_bids/path.py @@ -965,7 +965,7 @@ def update(self, *, check=None, **kwargs): _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})") - else: + elif isinstance(val, int): kwargs[key] = f"{val}" # ensure extension starts with a '.' From 5c68b1470131cdd74729ac4c84363fe0dae1a69e Mon Sep 17 00:00:00 2001 From: Stefan Appelhoff Date: Thu, 22 Feb 2024 11:06:57 +0100 Subject: [PATCH 07/10] revert some changes --- mne_bids/path.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mne_bids/path.py b/mne_bids/path.py index 39e6d5bb1..64ad6a1db 100644 --- a/mne_bids/path.py +++ b/mne_bids/path.py @@ -966,7 +966,7 @@ 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}" + kwargs[key] = f"{val:02}" # ensure extension starts with a '.' extension = kwargs.get("extension") From 197a7df08cc1d78888d204fd750ce9af9af3154e Mon Sep 17 00:00:00 2001 From: Stefan Appelhoff Date: Thu, 22 Feb 2024 11:10:53 +0100 Subject: [PATCH 08/10] update changelog --- doc/whats_new.rst | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/doc/whats_new.rst b/doc/whats_new.rst index 15afb5939..7ff39e8e5 100644 --- a/doc/whats_new.rst +++ b/doc/whats_new.rst @@ -40,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 ^^^^^^^^^^^^^^^ @@ -53,7 +57,6 @@ Detailed list of changes 🪲 Bug fixes ^^^^^^^^^^^^ -- Allow integer types to be passed to :class:`mne_bids.BIDSPath`, by `Alex Rockhill`_ (:gh:`1215`) - The datatype in the dataframe returned by :func:`mne_bids.stats.count_events` is now ``pandas.Int64Dtype`` instead of ``float64``, by `Eric Larson`_ (:gh:`1227`) From e54ee3283828109f67e7269833a64c18386fa63c Mon Sep 17 00:00:00 2001 From: Stefan Appelhoff Date: Thu, 22 Feb 2024 11:13:19 +0100 Subject: [PATCH 09/10] remove zfill again --- mne_bids/path.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mne_bids/path.py b/mne_bids/path.py index 64ad6a1db..39e6d5bb1 100644 --- a/mne_bids/path.py +++ b/mne_bids/path.py @@ -966,7 +966,7 @@ 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") From 6c48c6d9bf6701ac6e5cac66d9afa4e464382847 Mon Sep 17 00:00:00 2001 From: Stefan Appelhoff Date: Thu, 22 Feb 2024 11:13:49 +0100 Subject: [PATCH 10/10] bring back test --- mne_bids/tests/test_path.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mne_bids/tests/test_path.py b/mne_bids/tests/test_path.py index 7134fd9a7..efe449c7c 100644 --- a/mne_bids/tests/test_path.py +++ b/mne_bids/tests/test_path.py @@ -1342,6 +1342,9 @@ 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")