Skip to content
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

Update function following spikeinterface upgrade #4

Merged
merged 17 commits into from
Nov 20, 2024
Merged

Update function following spikeinterface upgrade #4

merged 17 commits into from
Nov 20, 2024

Conversation

ZhixiaoSu
Copy link
Collaborator

@ZhixiaoSu ZhixiaoSu commented Nov 19, 2024

  1. Updated package to take care of version difference between new and old versions of spikeinteraface and firmware.
  • 'openephys' changed to 'openephysbinary' in get_neo_streams

  • spikeinterface version changed to 0.101.2 in pyproject

  1. Misc
  • Changed saving processing log to inside of function generate_qc_report, so that calling the function itself (instead of the whole script) will also save processing log.
  • Updated driftmap.json so that use can decide if to perform phaseshift or not. (Very time consuming.)

@ZhixiaoSu ZhixiaoSu self-assigned this Nov 19, 2024
@@ -190,6 +190,7 @@ def plot_drift(directory, stream_name, block_index=0):
"vmax": 0,
"cmap": "Greys_r",
"figsize": [10, 10],
"phase_shift": {"margin_ms": 100.0},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe you can make phase_shift optional:

  • if "phase_shift" is None, don't run it
  • else, run it with params

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in the new version by adding an editable .json file.

@@ -0,0 +1,83 @@
{
"cells": [
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was this file added on purpose?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed this test file. It was added by accident.

@ZhixiaoSu ZhixiaoSu marked this pull request as ready for review November 19, 2024 19:58
@jsiegle
Copy link
Collaborator

jsiegle commented Nov 20, 2024

This looks good, merging into main!

@jsiegle jsiegle merged commit 14dce99 into main Nov 20, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants