From ecf0aaaa7489a54ac1cf7781f6b5ba68711d9fe4 Mon Sep 17 00:00:00 2001 From: Mathieu Scheltienne Date: Wed, 22 Feb 2023 16:15:25 +0100 Subject: [PATCH 1/8] deprecate 'kind' and 'path' from the layout reader --- mne/channels/layout.py | 75 +++++++++++++++++++++++-------- mne/channels/tests/test_layout.py | 50 +++++++++++++++++---- 2 files changed, 98 insertions(+), 27 deletions(-) diff --git a/mne/channels/layout.py b/mne/channels/layout.py index 20b99656a7c..bef6a391375 100644 --- a/mne/channels/layout.py +++ b/mne/channels/layout.py @@ -163,18 +163,30 @@ def _read_lay(fname): return box, pos, names, ids -def read_layout(kind, path=None, scale=True): +def read_layout(kind=None, path=None, fname=None, *, scale=True): """Read layout from a file. Parameters ---------- - kind : str + kind : str | None The name of the ``.lout`` file (e.g. ``kind='Vectorview-all'`` for ``'Vectorview-all.lout'``). + + .. deprecated:: v1.4 + The ``kind`` and ``path`` parameters will be removed in version + 1.5. Please use the ``fname`` parameter instead. path : path-like | None The path of the folder containing the Layout file. Defaults to the ``mne/channels/data/layouts`` folder inside your mne-python installation. + + .. deprecated:: v1.4 + The ``kind`` and ``path`` parameters will be removed in version + 1.5. Please use the ``fname`` parameter instead. + fname : path-like | str + Either the path to a ``.lout`` or ``.lay`` file or the name of a + built-in layout. c.f. Notes for a list of the available built-in + layouts. scale : bool Apply useful scaling for out the box plotting using ``layout.pos``. Defaults to True. @@ -190,7 +202,7 @@ def read_layout(kind, path=None, scale=True): Notes ----- - Valid ``kind`` arguments are: + Valid ``fname`` (or ``kind``) arguments are: .. table:: :widths: auto @@ -241,22 +253,47 @@ def read_layout(kind, path=None, scale=True): """ readers = {".lout": _read_lout, ".lay": _read_lay} - if path is None: - path = Path(__file__).parent / "data" / "layouts" - # kind should be the name as a string, but let's consider the case where - # the path to the file is provided instead. - kind = Path(kind) - if len(kind.suffix) == 0 and (path / kind.with_suffix(".lout")).exists(): - kind = kind.with_suffix(".lout") - elif len(kind.suffix) == 0 and (path / kind.with_suffix(".lay")).exists(): - kind = kind.with_suffix(".lay") - - fname = kind if kind.exists() else path / kind.name - if fname.suffix not in (".lout", ".lay"): - raise ValueError( - "Unknown layout type. Should be of type .lout or .lay." + if fname is None: # deprecated in 1.4 + warn( + "Argument 'kind' and 'path' are deprecated in favor of 'fname'.", + DeprecationWarning, ) - kind = fname.stem + if path is None: + path = Path(__file__).parent / "data" / "layouts" + # kind should be the name as a string, but let's consider the case where + # the path to the file is provided instead. + kind = Path(kind) + if len(kind.suffix) == 0 and (path / kind.with_suffix(".lout")).exists(): + kind = kind.with_suffix(".lout") + elif len(kind.suffix) == 0 and (path / kind.with_suffix(".lay")).exists(): + kind = kind.with_suffix(".lay") + + fname = kind if kind.exists() else path / kind.name + if fname.suffix not in (".lout", ".lay"): + raise ValueError( + "Unknown layout type. Should be of type .lout or .lay." + ) + kind = fname.stem + else: + if kind is not None: # to be removed along the deprecated argument + raise ValueError( + "Argument 'kind' and 'path' should be None if 'fname' is used." + ) + if isinstance(fname, str): + # is it a built-in layout? + directory = Path(__file__).parent / "data" / "layouts" + if (directory / fname).exists(): + fname = directory / fname + elif (directory / fname).with_suffix(".lout").exists(): + fname = (directory / fname).with_suffix(".lout") + elif (directory / fname).with_suffix(".lay").exists(): + fname = (directory / fname).with_suffix(".lay") + # if not, it must be a valid path provided as str or Path + fname = _check_fname( + fname, "read", must_exist=True, name="layout" + ) + kind = fname.stem + box, pos, names, ids = readers[fname.suffix](fname) if scale: @@ -495,7 +532,7 @@ def find_layout(info, ch_type=None, exclude='bads'): return generate_2d_layout(xy, ch_names=ch_names, name='custom', normalize=True) - layout = read_layout(layout_name) + layout = read_layout(fname=layout_name) if not is_old_vv: layout.names = _clean_names(layout.names, remove_whitespace=True) if has_CTF_grad: diff --git a/mne/channels/tests/test_layout.py b/mne/channels/tests/test_layout.py index 7956c3b1da2..99151a43047 100644 --- a/mne/channels/tests/test_layout.py +++ b/mne/channels/tests/test_layout.py @@ -54,22 +54,44 @@ def _get_test_info(): def test_io_layout_lout(tmp_path): """Test IO with .lout files.""" - layout = read_layout('Vectorview-all', scale=False) + with pytest.warns( + DeprecationWarning, match="'kind' and 'path' are deprecated" + ): + layout = read_layout('Vectorview-all', scale=False) layout.save(tmp_path / "foobar.lout") + with pytest.warns( + DeprecationWarning, match="'kind' and 'path' are deprecated" + ): + layout_read = read_layout( + tmp_path / "foobar.lout", path="./", scale=False + ) + assert_array_almost_equal(layout.pos, layout_read.pos, decimal=2) + assert layout.names == layout_read.names + assert " Date: Wed, 22 Feb 2023 16:17:00 +0100 Subject: [PATCH 2/8] add entry to changelog --- doc/changes/latest.inc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/changes/latest.inc b/doc/changes/latest.inc index 727055ac6c7..e819524ec10 100644 --- a/doc/changes/latest.inc +++ b/doc/changes/latest.inc @@ -54,4 +54,4 @@ Bugs API changes ~~~~~~~~~~~ -- None yet +- Deprecate arguments ``kind`` and ``path`` from `mne.channels.read_layout` in favor of a common argument ``fname`` (:gh:`11500` by `Mathieu Scheltienne`_) From dbc038676d70902a2c6b2b336e5f1f8aeb133661 Mon Sep 17 00:00:00 2001 From: Mathieu Scheltienne Date: Wed, 22 Feb 2023 16:23:40 +0100 Subject: [PATCH 3/8] fix flake8 --- mne/channels/layout.py | 19 +++++++++++-------- mne/channels/tests/test_layout.py | 2 +- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/mne/channels/layout.py b/mne/channels/layout.py index bef6a391375..69fd69a9891 100644 --- a/mne/channels/layout.py +++ b/mne/channels/layout.py @@ -260,12 +260,18 @@ def read_layout(kind=None, path=None, fname=None, *, scale=True): ) if path is None: path = Path(__file__).parent / "data" / "layouts" - # kind should be the name as a string, but let's consider the case where - # the path to the file is provided instead. + # kind should be the name as a string, but let's consider the case + # where the path to the file is provided instead. kind = Path(kind) - if len(kind.suffix) == 0 and (path / kind.with_suffix(".lout")).exists(): + if ( + len(kind.suffix) == 0 + and (path / kind.with_suffix(".lout")).exists() + ): kind = kind.with_suffix(".lout") - elif len(kind.suffix) == 0 and (path / kind.with_suffix(".lay")).exists(): + elif ( + len(kind.suffix) == 0 + and (path / kind.with_suffix(".lay")).exists() + ): kind = kind.with_suffix(".lay") fname = kind if kind.exists() else path / kind.name @@ -289,11 +295,8 @@ def read_layout(kind=None, path=None, fname=None, *, scale=True): elif (directory / fname).with_suffix(".lay").exists(): fname = (directory / fname).with_suffix(".lay") # if not, it must be a valid path provided as str or Path - fname = _check_fname( - fname, "read", must_exist=True, name="layout" - ) + fname = _check_fname(fname, "read", must_exist=True, name="layout") kind = fname.stem - box, pos, names, ids = readers[fname.suffix](fname) if scale: diff --git a/mne/channels/tests/test_layout.py b/mne/channels/tests/test_layout.py index 99151a43047..c17e3ccc038 100644 --- a/mne/channels/tests/test_layout.py +++ b/mne/channels/tests/test_layout.py @@ -91,7 +91,7 @@ def test_io_layout_lay(tmp_path): layout = read_layout(fname="CTF151", scale=False) layout.save(str(tmp_path / "foobar.lay")) layout_read = read_layout( - fname = tmp_path / "foobar.lay", scale=False + fname=tmp_path / "foobar.lay", scale=False ) assert_array_almost_equal(layout.pos, layout_read.pos, decimal=2) assert layout.names == layout_read.names From 4d64405a353cca10422ebbff278a3625fe95b00a Mon Sep 17 00:00:00 2001 From: Mathieu Scheltienne Date: Mon, 27 Feb 2023 14:17:13 +0100 Subject: [PATCH 4/8] better deprecation --- mne/channels/layout.py | 36 ++++++++++++++++--------------- mne/channels/tests/test_layout.py | 28 +++++++++--------------- 2 files changed, 29 insertions(+), 35 deletions(-) diff --git a/mne/channels/layout.py b/mne/channels/layout.py index ed6103d454c..5107cf69891 100644 --- a/mne/channels/layout.py +++ b/mne/channels/layout.py @@ -163,18 +163,15 @@ def _read_lay(fname): return box, pos, names, ids -def read_layout(kind=None, path=None, fname=None, *, scale=True): +def read_layout(fname=None, path="", scale=True, *, kind=None): """Read layout from a file. Parameters ---------- - kind : str | None - The name of the ``.lout`` file (e.g. ``kind='Vectorview-all'`` for - ``'Vectorview-all.lout'``). - - .. deprecated:: v1.4 - The ``kind`` and ``path`` parameters will be removed in version - 1.5. Please use the ``fname`` parameter instead. + fname : path-like | str + Either the path to a ``.lout`` or ``.lay`` file or the name of a + built-in layout. c.f. Notes for a list of the available built-in + layouts. path : path-like | None The path of the folder containing the Layout file. Defaults to the ``mne/channels/data/layouts`` folder inside your mne-python @@ -183,13 +180,16 @@ def read_layout(kind=None, path=None, fname=None, *, scale=True): .. deprecated:: v1.4 The ``kind`` and ``path`` parameters will be removed in version 1.5. Please use the ``fname`` parameter instead. - fname : path-like | str - Either the path to a ``.lout`` or ``.lay`` file or the name of a - built-in layout. c.f. Notes for a list of the available built-in - layouts. scale : bool Apply useful scaling for out the box plotting using ``layout.pos``. Defaults to True. + kind : str | None + The name of the ``.lout`` file (e.g. ``kind='Vectorview-all'`` for + ``'Vectorview-all.lout'``). + + .. deprecated:: v1.4 + The ``kind`` and ``path`` parameters will be removed in version + 1.5. Please use the ``fname`` parameter instead. Returns ------- @@ -202,7 +202,7 @@ def read_layout(kind=None, path=None, fname=None, *, scale=True): Notes ----- - Valid ``fname`` (or ``kind``) arguments are: + Valid ``fname`` arguments are: .. table:: :widths: auto @@ -258,7 +258,7 @@ def read_layout(kind=None, path=None, fname=None, *, scale=True): "Argument 'kind' and 'path' are deprecated in favor of 'fname'.", DeprecationWarning, ) - if path is None: + if path == "" or path is None: path = Path(__file__).parent / "data" / "layouts" # kind should be the name as a string, but let's consider the case # where the path to the file is provided instead. @@ -281,9 +281,11 @@ def read_layout(kind=None, path=None, fname=None, *, scale=True): ) kind = fname.stem else: - if kind is not None: # to be removed along the deprecated argument - raise ValueError( - "Argument 'kind' and 'path' should be None if 'fname' is used." + if kind is not None or path != "": # to be removed along the deprecated argument + warn( + "Argument 'kind' and 'path' are deprecated in favor of " + "'fname' and should not be provided alongside 'fname'.", + DeprecationWarning, ) if isinstance(fname, str): # is it a built-in layout? diff --git a/mne/channels/tests/test_layout.py b/mne/channels/tests/test_layout.py index c17e3ccc038..acb597eb886 100644 --- a/mne/channels/tests/test_layout.py +++ b/mne/channels/tests/test_layout.py @@ -54,22 +54,6 @@ def _get_test_info(): def test_io_layout_lout(tmp_path): """Test IO with .lout files.""" - with pytest.warns( - DeprecationWarning, match="'kind' and 'path' are deprecated" - ): - layout = read_layout('Vectorview-all', scale=False) - layout.save(tmp_path / "foobar.lout") - with pytest.warns( - DeprecationWarning, match="'kind' and 'path' are deprecated" - ): - layout_read = read_layout( - tmp_path / "foobar.lout", path="./", scale=False - ) - assert_array_almost_equal(layout.pos, layout_read.pos, decimal=2) - assert layout.names == layout_read.names - assert " Date: Mon, 27 Feb 2023 14:21:20 +0100 Subject: [PATCH 5/8] check the extension --- mne/channels/layout.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mne/channels/layout.py b/mne/channels/layout.py index 5107cf69891..f071a4aadd7 100644 --- a/mne/channels/layout.py +++ b/mne/channels/layout.py @@ -298,6 +298,8 @@ def read_layout(fname=None, path="", scale=True, *, kind=None): fname = (directory / fname).with_suffix(".lay") # if not, it must be a valid path provided as str or Path fname = _check_fname(fname, "read", must_exist=True, name="layout") + # and it must have a valid extension + _check_option("fname extension", fname.suffix, readers) kind = fname.stem box, pos, names, ids = readers[fname.suffix](fname) From 8517a2437187461ac6673a039da4dded490b8193 Mon Sep 17 00:00:00 2001 From: Mathieu Scheltienne Date: Mon, 27 Feb 2023 14:25:26 +0100 Subject: [PATCH 6/8] fix layout tests --- mne/channels/tests/test_layout.py | 40 ++++++++++--------------------- 1 file changed, 13 insertions(+), 27 deletions(-) diff --git a/mne/channels/tests/test_layout.py b/mne/channels/tests/test_layout.py index acb597eb886..e17f90cafaf 100644 --- a/mne/channels/tests/test_layout.py +++ b/mne/channels/tests/test_layout.py @@ -155,23 +155,17 @@ def test_find_topomap_coords(): def test_make_eeg_layout(tmp_path): """Test creation of EEG layout.""" - tmp_name = 'foo' - lout_name = 'test_raw' - with pytest.warns( - DeprecationWarning, match="'kind' and 'path' are deprecated" - ): - lout_orig = read_layout(kind=lout_name, path=lout_path) + lout_orig = read_layout(fname=lout_path / "test_raw.lout") info = read_info(fif_fname) - info['bads'].append(info['ch_names'][360]) + info["bads"].append(info["ch_names"][360]) layout = make_eeg_layout(info, exclude=[]) - assert_array_equal(len(layout.names), len([ch for ch in info['ch_names'] - if ch.startswith('EE')])) - layout.save(str(tmp_path / (tmp_name + ".lout"))) - with pytest.warns( - DeprecationWarning, match="'kind' and 'path' are deprecated" - ): - lout_new = read_layout(kind=tmp_name, path=tmp_path, scale=False) - assert_array_equal(lout_new.kind, tmp_name) + assert_array_equal( + len(layout.names), + len([ch for ch in info["ch_names"] if ch.startswith("EE")]), + ) + layout.save(str(tmp_path / "foo.lout")) + lout_new = read_layout(fname=tmp_path / "foo.lout", scale=False) + assert_array_equal(lout_new.kind, "foo") assert_allclose(layout.pos, lout_new.pos, atol=0.1) assert_array_equal(lout_orig.names, lout_new.names) @@ -186,19 +180,11 @@ def test_make_eeg_layout(tmp_path): def test_make_grid_layout(tmp_path): """Test creation of grid layout.""" - tmp_name = 'bar' - lout_name = 'test_ica' - with pytest.warns( - DeprecationWarning, match="'kind' and 'path' are deprecated" - ): - lout_orig = read_layout(kind=lout_name, path=lout_path) + lout_orig = read_layout(fname=lout_path / "test_ica.lout") layout = make_grid_layout(_get_test_info()) - layout.save(str(tmp_path / (tmp_name + ".lout"))) - with pytest.warns( - DeprecationWarning, match="'kind' and 'path' are deprecated" - ): - lout_new = read_layout(kind=tmp_name, path=tmp_path) - assert_array_equal(lout_new.kind, tmp_name) + layout.save(str(tmp_path / "bar.lout")) + lout_new = read_layout(fname=tmp_path / "bar.lout") + assert_array_equal(lout_new.kind, "bar") assert_array_equal(lout_orig.pos, lout_new.pos) assert_array_equal(lout_orig.names, lout_new.names) From ae93ab3447626e4efb98002cbf3e393bb19ce8ab Mon Sep 17 00:00:00 2001 From: Mathieu Scheltienne Date: Mon, 27 Feb 2023 14:30:23 +0100 Subject: [PATCH 7/8] fix flake8 --- mne/channels/layout.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mne/channels/layout.py b/mne/channels/layout.py index f071a4aadd7..87149f458a4 100644 --- a/mne/channels/layout.py +++ b/mne/channels/layout.py @@ -281,7 +281,8 @@ def read_layout(fname=None, path="", scale=True, *, kind=None): ) kind = fname.stem else: - if kind is not None or path != "": # to be removed along the deprecated argument + # to be removed along the deprecated argument + if kind is not None or path != "": warn( "Argument 'kind' and 'path' are deprecated in favor of " "'fname' and should not be provided alongside 'fname'.", From 8e9c3f04c0f95b10da0df3b5a3f36c6d07489a9c Mon Sep 17 00:00:00 2001 From: Eric Larson Date: Mon, 27 Feb 2023 09:48:07 -0500 Subject: [PATCH 8/8] Update doc/changes/latest.inc --- doc/changes/latest.inc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/changes/latest.inc b/doc/changes/latest.inc index d13d41f0321..18f5c7e29e7 100644 --- a/doc/changes/latest.inc +++ b/doc/changes/latest.inc @@ -56,4 +56,4 @@ Bugs API changes ~~~~~~~~~~~ -- Deprecate arguments ``kind`` and ``path`` from `mne.channels.read_layout` in favor of a common argument ``fname`` (:gh:`11500` by `Mathieu Scheltienne`_) +- Deprecate arguments ``kind`` and ``path`` from :func:`mne.channels.read_layout` in favor of a common argument ``fname`` (:gh:`11500` by `Mathieu Scheltienne`_)