num_chan to num_channels in BinaryRecordingExtractor
#1754
Conversation
|
Good idea. And lets keep the wrning for a while because we have so many code with the old variable name.... |
| is_filtered=None, | ||
| num_chan=None, | ||
| ): | ||
| num_channels = num_channels or num_chan |
There was a problem hiding this comment.
What if they are both given? and maybe different? e.g. num_channels=16 and num_chan=32?
There was a problem hiding this comment.
This:
https://en.wikipedia.org/wiki/Short-circuit_evaluation
Together with this:
https://docs.python.org/3/library/stdtypes.html#truth-value-testing
It is a common python idiom but maybe I should make the point of avoiding it as it can be confusing.
alejoe91
left a comment
alejoe91
left a comment
There was a problem hiding this comment.
@h-mayorquin thanks for this! I have a small comment: IMO we should make the init checks even more stringent! Currently, one could pass both arguments and in that case the behavior is not clear
3 participants
While I was working on #1742 I realized that the
BinaryRecordingExtractordoes not follow the convention of the library of referring to number of channels asnum_channels(as we do inBaseRecording.get_num_channels). Instead, it usesnum_chan.This PR starts adding a new argument
BinaryRecordingExtractorand will throw a deprecation warning for the old argument.