Add Keywords to the Documentation Functions #2057
Conversation
for more information, see https://pre-commit.ci
|
|
||
| cs = CurationSorting(sorting) | ||
| cs = CurationSorting(parent_sorting=sorting) |
There was a problem hiding this comment.
Is there a reason for this? Why do some functions use parent_sorting or parent_recording and most use sorting or recording.
There was a problem hiding this comment.
good point, we should unify this indeed! What would you vote for? Probably just recording and sorting?
There was a problem hiding this comment.
Yeah, I think the path of least resistance and the most obvious for the user is just to use recording and sorting. I don't think I would guess that I have a parent_sorting vs child_sorting at the api level for most functions. (Also less typing).
There was a problem hiding this comment.
Agreed. It's also a good time to make the switch. Do you think we should deprecate the old arguments or just break compatibility towards the next release?
There was a problem hiding this comment.
There was a problem hiding this comment.
well this is only changing an argument name that it's probably not really used by most users. I think that most users will use banpass_filter(recording) and not bandpass_filter(parent_recording=recording) (same for channel_slice, other preprocessors, etc..). Keeping back-compatibility would mean adding a temporary argument and setting the main recording to None, which would possibly require to make some other args None by default...
Let's just make the switch. I checked and it's not that many files :)
Co-authored-by: Alessio Buccino <[email protected]>
As discussed in #2046.
I didn't really touch the imports ( I only added imports in a couple spots where it was missing). Once we settle on that I'll just do that in a separate PR.