Skip to content

Conversation

@mscheltienne
Copy link
Member

Follow up on the discussion in #10035

@mscheltienne mscheltienne marked this pull request as draft November 25, 2021 12:26
@mscheltienne mscheltienne marked this pull request as ready for review November 26, 2021 14:06
@mscheltienne mscheltienne changed the title Deprecrate pick channels in info MRG: Deprecrate pick channels in info Nov 26, 2021
@mscheltienne
Copy link
Member Author

To give some context, I wanted to see if equalize_channels had to operate on Info as well. It breaks a lot of stuff if it doesn't; so I ended up adding the selection of channels for Info directly in equalize_channels instead of using Info.pick_channels.

@mscheltienne
Copy link
Member Author

Doc build failure doesn't seem to be related to this PR.

Copy link
Member

@larsoner larsoner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some minor stuff, thanks for taking care of this @mscheltienne !

mscheltienne and others added 2 commits November 28, 2021 21:49
Co-authored-by: Eric Larson <larson.eric.d@gmail.com>
@mscheltienne
Copy link
Member Author

Thanks for the tips, let's see if CIs are green; I wasn't quite sure on how the deprecated decorator works (especially as it doesn't throw any warning at me in the IPython console).

self['ch_names'] = [ch['ch_name'] for ch in self['chs']]
self['nchan'] = len(self['chs'])

@deprecated('use inst.pick_channels instead.')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should just automagically work. If it doesn't, I'll push a commit to fix the decorator (it's possible it has some bug)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CIs are happy, so all good I guess, but I don't get any warning issued when using a function decorated with @deprecated in python console or in Spyder's IPython console. The docstring is edited however with .. warning:: DEPRECATED:. Is this the correct behavior for this decorator?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot replicate; on your branch I get:

>>> import mne
>>> import warnings
>>> warnings.simplefilter('always', DeprecationWarning)
>>> info = mne.create_info(2, 1000., 'eeg')
>>> info.pick_channels(info['ch_names'][:1])
<decorator-gen-25>:4: DeprecationWarning: Function pick_channels is deprecated; use inst.pick_channels instead.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Getting the warning as well on this mac laptop.. I guess the problem is with my windows computer and the venv.

@larsoner larsoner merged commit 602958d into mne-tools:main Nov 29, 2021
@larsoner
Copy link
Member

Thanks @mscheltienne !

@mscheltienne mscheltienne deleted the deprecrate_pick_channels_in_info branch November 29, 2021 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants