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

Open folders with user altered filenames in SpikeGLX #1608

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

h-mayorquin
Copy link
Contributor

@h-mayorquin h-mayorquin commented Dec 9, 2024

Should fix #1606

To solve #1606 this does two things:

  1. It calculates device (probe) index from the metadata instead of relying on file parsing as we currently do.
  2. When the gate, port and dock are not available in the metadata it parses the filename as named in the metadata (as that was the original one) instead of from the filepath in the system as we are currently doing.

Both things should make the reader more robust.

I am preparing some data for gin where the filename was changed. I will change this from draft once I add that.

neo/rawio/spikeglxrawio.py Outdated Show resolved Hide resolved
@h-mayorquin h-mayorquin marked this pull request as ready for review December 10, 2024 00:48
@h-mayorquin
Copy link
Contributor Author

OK, I added the tests and this should be ready for review.

Copy link
Contributor Author

@h-mayorquin h-mayorquin left a comment

Choose a reason for hiding this comment

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

Added comments to hopefully make the reviewing easier.

@@ -82,14 +82,11 @@ class SpikeGLXRawIO(BaseRawWithBufferApiIO):

Notes
-----
* Contrary to other implementations this IO reads the entire folder and subfolders and:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed this because it is not clear what other implementations this is referring to, probably the ones before but then I don't think it should be here.

* For imec device both "ap" and "lf" are extracted so one device have several "streams"
* There are several versions depending the neuropixel probe generation (`1.x`/`2.x`/`3.x`)
* Here, we assume that the `meta` file has the same structure across all generations.
* This IO is developed based on neuropixel generation 2.0, single shank recordings.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is a note of the first version but we have gradually added support. This supports the shanked version just fine and also NHP and other varieties.

* Here, we assume that the `meta` file has the same structure across all generations.
* This IO is developed based on neuropixel generation 2.0, single shank recordings.
* This IO reads the entire folder and subfolders locating the `.bin` and `.meta` files
* Handles gates and triggers as segments (based on the `_gt0`, `_gt1`, `_t0` , `_t1` in filenames)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added specific mention to gates and triggers as that is more in line with SpikeGLX documentation.


order_key = list({make_key(info) for info in info_list})
order_key = sorted(order_key)
# This sets non-integers values before integers
Copy link
Contributor Author

Choose a reason for hiding this comment

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

So this section is the core of the PR. It uses the same mechanism that we have for gates and triggers before but also for probe index.:

  • It creates a set of unique combinations of (gate, trigger) and (probe, port,dock).
  • That combination is used to create the index (segment index and probe index)
  • The parameters of info are used to assign the segment and probe index with a dict.

The normalize lambda unifies this behavior. Maybe there is a better naming but I could not come with anything, it just follows the convention that Sam had before.


# Define stream base on device [imec|nidq], device_index and stream_kind [ap|lf] for imec
for info in info_list:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This defines the stream here after all the information is available. Previously, it was done inside the extract_info_function.

@@ -488,13 +513,15 @@ def extract_stream_info(meta_file, meta):
else:
# NIDQ case
has_sync_trace = False
fname = Path(meta_file).stem

bin_file_path = meta["fileName"]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the part that uses the meta["fileName"] (the original one) instead of the file_path in the system (which the user might have changed).

@@ -563,12 +593,15 @@ def extract_stream_info(meta_file, meta):
info["trigger_num"] = trigger_num
info["device"] = device
info["stream_kind"] = stream_kind
info["stream_name"] = stream_name
info["device_kind"] = meta.get("typeThis", device.split(".")[0])
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Get the device kind directly from the metadata. imec or nidq. All non-production probes should have this in their metadata. For production probes this would rely on file parsing as we were doing before:

https://billkarsh.github.io/SpikeGLX/help/parsing/

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.

Unable to open folders with user altered filenames in SpikeGLX
3 participants