Fix Kilosort Phy reader docstrings #2022
Conversation
alejoe91
added
documentation
| exclude_cluster_groups: list or str, optional | ||
| Cluster groups to exclude (e.g. "noise" or ["noise", "mua"]). |
There was a problem hiding this comment.
| exclude_cluster_groups: list or str, optional | |
| Cluster groups to exclude (e.g. "noise" or ["noise", "mua"]). | |
| exclude_cluster_groups: list or str, default: None | |
| Cluster groups to exclude (e.g. "noise" or ["noise", "mua"]). | |
| If None keeps all cluster groups |
Did you want to start switching away from optional and include default: None in this case?
There was a problem hiding this comment.
yes! let me do it since I'm at it :)
There was a problem hiding this comment.
@zm711 should be good to go. Also added typing
There was a problem hiding this comment.
@alejoe91 looks good to me. Also with the new typing (although it is a bit annoying/also very Python) that you can also do list[str] and use the built in type rather than the typing List. That's not a suggestion, it's more of a rant that sometimes a little less flexibility makes it easier for programmers to be consistent. :)
There was a problem hiding this comment.
It fails with python 3.8 though! So as long as we keep support for 3.8 we should stick to this IMO :)
There was a problem hiding this comment.
good point. I'm sure @h-mayorquin would know :)
There was a problem hiding this comment.
Now that's the scientist: just do the test and prove the hypothesis :)
There was a problem hiding this comment.
Now that's the scientist: just do the test and prove the hypothesis :)
I wish science was that easy!!! :)
There was a problem hiding this comment.
Empiricism for the win!
Fixes #2020