Allow export_to_phy to work with fast_templates
#2549
Conversation
|
Salut Aurelien. |
|
Done! I also implemented a function |
for more information, see https://pre-commit.ci
|
This is OK for me. |
|
|
||
| Parameters | ||
| ---------- | ||
| unit_id: |
There was a problem hiding this comment.
Well it depends,
There is currently no defined type for unit_id in SpikeInterface
There was a problem hiding this comment.
Aren't they strings | int. Right?
There was a problem hiding this comment.
I believe they are, but who says that won't change in the future?
There was a problem hiding this comment.
You. Right now. Lock it in and take control. :)
There was a problem hiding this comment.
str | int
Alessio is fighting for str only but I strongly resist.
Co-authored-by: Zach McKenzie <92116279+zm711@users.noreply.github.com>
Co-authored-by: Zach McKenzie <92116279+zm711@users.noreply.github.com>
Depends on SpikeInterface#2549 (which needs to be merged before)
| assert ( | ||
| templates_ext is not None | ||
| ), "export_to_phy requires SortingAnalyzer with either extension 'templates' or 'fast_templates'" |
There was a problem hiding this comment.
I was thinking about this and templates_ext cannot be None, because if it is None above the templates_ext.get_templates will fail. Or am I misreading this? It seems to me that it would be better to do a check for none after line 185. Something like
if templates_ext is None:
raise ValueError('Must have either calculated templates or fast_templates')That way regardless of if the | notation is accepted you have the check in place.
There was a problem hiding this comment.
True, nice catch!
But as you said, I'm waiting for the | PR to be merged, to re-write this more cleanly :)
|
Ok I updated the implementation to be cleaner, and follow what we agreed to follow in #2570 This is ready on my end :) |
for more information, see https://pre-commit.ci
|
@alejoe91 The test failed for some weird reason (not my fault), and I don't have the permissions to re-launch the test :/ |
| @@ -180,8 +180,10 @@ def export_to_phy( | |||
|
|
|||
| # export templates/templates_ind/similar_templates | |||
| # shape (num_units, num_samples, max_num_channels) | |||
| templates_ext = sorting_analyzer.get_extension("templates") | |||
| templates_ext is not None, "export_to_phy need SortingAnalyzer with extension 'templates'" | |||
| templates_ext = sorting_analyzer.get_extension("templates") or sorting_analyzer.get_extension("fast_templates") | |||
No description provided.