-
Notifications
You must be signed in to change notification settings - Fork 5
Port SpikeInterface update for tutorial generation #917
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Looks like GitHubs just having bad service all around today |
|
@alejoe91 Looks like we get this error: https://github.com/NeurodataWithoutBorders/nwb-guide/actions/runs/10491656279/job/29061326476?pr=917#step:13:178 Traceback (most recent call last):
File "/home/runner/miniconda3/envs/nwb-guide/lib/python3.9/site-packages/flask/app.py", line 1484, in full_dispatch_request
rv = self.dispatch_request()
File "/home/runner/miniconda3/envs/nwb-guide/lib/python3.9/site-packages/flask/app.py", line 1469, in dispatch_request
return self.ensure_sync(self.view_functions[rule.endpoint])(**view_args)
File "/home/runner/miniconda3/envs/nwb-guide/lib/python3.9/site-packages/flask_restx/api.py", line 404, in wrapper
resp = resource(*args, **kwargs)
File "/home/runner/miniconda3/envs/nwb-guide/lib/python3.9/site-packages/flask/views.py", line 109, in view
return current_app.ensure_sync(self.dispatch_request)(**kwargs)
File "/home/runner/miniconda3/envs/nwb-guide/lib/python3.9/site-packages/flask_restx/resource.py", line 46, in dispatch_request
resp = meth(*args, **kwargs)
File "/home/runner/work/nwb-guide/nwb-guide/src/pyflask/namespaces/data.py", line 19, in post
generate_test_data(output_path=arguments["output_path"])
File "/home/runner/work/nwb-guide/nwb-guide/src/pyflask/manageNeuroconv/manage_neuroconv.py", line [173](https://github.com/NeurodataWithoutBorders/nwb-guide/actions/runs/10491656279/job/29061326476?pr=917#step:13:174)6, in generate_test_data
spikeinterface.exporters.export_to_phy(
File "/home/runner/miniconda3/envs/nwb-guide/lib/python3.9/site-packages/spikeinterface/exporters/to_phy.py", line 184, in export_to_phy
assert templates_ext is not None, "export_to_phy requires a SortingAnalyzer with the extension 'templates'"
AssertionError: export_to_phy requires a SortingAnalyzer with the extension 'templates'Were you able to run the isolated helper function successfully on your end? (like, just a copy/paste into ipython or similar?) |
|
@alejoe91 Any ideas? |
|
@alejoe91 ping |
|
DevTests pass but tutorial data generation is failing due to NeuroConv's NWBMetaDataEncoder should already handle cases of scalar int64 and an array of int64. Not sure what is going on here. Compound types? Will debug locally when I have time. |
|
Tracking updates here. I updated neuroconv to 0.6.0 instead of the latest to make upgrading neuroconv here more manageable. 0.6.0 was the next release of neuroconv after the currently pinned commit. This version of neuroconv requires spikeinterface>=0.101.0 which introduces the changes to SortingAnalyzer. I believe that should resolve the above issues. However, updating neuroconv to 0.6.0 results in other changes that need to be addressed. New interfaces were introduced. And the file/folder selector box on the source data page has changed to text boxes:
This is probably related to the 0.6.0 change:
The current macos-13 build test failure seems to be due to a failure in finding/linking the hdf5 libs. |
Attempt pytables/h5py hdf5 incompatibility
for more information, see https://pre-commit.ci
|
I resolved the incompatibility between hdf5 libs used by h5py and tables. The issues related to supporting neuroconv 0.6.0 beyond the spikeinterface update will be resolved in a separate PR. So all tests are green. Local tutorial run passes. This is ready for review. |
pauladkisson
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good

fix #914
replace #916
GitHub was being slow to change the merge target (took like 10 minutes to allow merge) so in less time than that I just ported over the essential changes
Let's see what CI says