Deprecate 'kind' and 'path' in favor of 'fname' in the layout reader#11500
Deprecate 'kind' and 'path' in favor of 'fname' in the layout reader#11500larsoner merged 9 commits intomne-tools:mainfrom
Conversation
mne/channels/layout.py
Outdated
|
|
||
|
|
||
| def read_layout(kind, path=None, scale=True): | ||
| def read_layout(kind=None, path=None, fname=None, *, scale=True): |
There was a problem hiding this comment.
-
All existing user code currently will have either
read_layout(name, ...)orread_layout(kind=name, ...). It would be nice if at least the first one (which might even be the most common!) continued to work without a deprecation warning if possible. So what if we deprecate differently by doing:Suggested changedef read_layout(kind=None, path=None, fname=None, *, scale=True): def read_layout(fname=None, path=None, scale=True, *, kind=None): if kind is not None: warn(...) ... if path is not None: warn(...) ... That way
read_layout('Vectorview-all')works on 1.3 and also in this PR without any warning. -
Since the current default for
pathisNone, we need to use something other thanNoneas the default now to know if a user has actually passed the param or not. otherwise code like this:read_layout('VectorView-all', path=None)works on 1.3, does not warn the user for passing
path=Nonein 1.4, yet will raise an error about the kwarg not existing come 1.5. We could for example usepath=''(I think it's safe to assume no user has donepath=''in their code) and do:_validate_type(path, ('path-like', None), 'path') if path != '': warn(...)as our user-passed-so-need-to-warn check.
There was a problem hiding this comment.
Good points, it should be better now, thanks!
|
I don't think the CI failure is related to the PR. |
larsoner
left a comment
There was a problem hiding this comment.
LGTM, will merge once CIs come back happy again (other than the notebook one, which I'll investigate separately)!
* upstream/main: (264 commits) BUG: Fix deprecated API usage in example (mne-tools#11512) Deprecate 'kind' and 'path' in favor of 'fname' in the layout reader (mne-tools#11500) EGI/MFF events outside EEG recording should not break the code (mne-tools#11505) fixed annotations error on export (mne-tools#11435) DOC: Update installer links [skip azp] [skip actions] [skip cirrus] (mne-tools#11506) BUG: updates for MPL 3.7 compatibility (mne-tools#11409) Fix docstrings by replacing str with path-like and fix double backticks for formatting (mne-tools#11499) Use pathlib.Path instead of os.path to handle files and folders [circle deploy] (mne-tools#11473) MAINT: Fix Circle [circle deploy] (mne-tools#11497) MAINT: Use mamba in CIs (mne-tools#11471) Updating documentation to clarify full vs half-bandwidth and defaults in time_frequency.multitaper.py (mne-tools#11479) Fix typo in tutorial (mne-tools#11498) Typo fix and added colons before code (mne-tools#11496) [MRG] ENH/DOC: demo custom spectrum creation (mne-tools#11493) Accept only left-clicks for adding annotations (mne-tools#11491) [BUG, MRG] Fix pial surface loading, logging in locate_ieeg (mne-tools#11489) [ENH] Added unit_role to add non-breaking space between magnitude and units (mne-tools#11469) MAINT: Fix CircleCI build (mne-tools#11488) [DOC] Updated decoding.SSD documentation and internal variable naming (mne-tools#11475) Typo fix (mne-tools#11485) ...
Closes #11483
As discussed in #11483, those 2 arguments don't play well and could be replaced by a common explicit
fnameargument.