Make _parse_imro_string self-contained by deriving IMRO field schemas from the catalogue#417
Open
h-mayorquin wants to merge 1 commit intoSpikeInterface:mainfrom
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is another PR in the series of Neuropixels cleanups (see #410, #405) with the goal of making everything come from a single source of data: the Neuropixels probe library from ProbeTable.
The following victim is
_parse_imro_string, which required callers to pass aprobe_part_numberjust so it could look up the IMRO field schema, even though the IMRO string itself contains the format type code needed to determine the schema. This forced us to maintain a hand-written dict (probe_part_number_to_probe_type) that duplicated information already present in the ProbeTable catalogue, andread_imrohad to pre-parse the header and reverse-lookup through it. The dict had to be manually updated whenever a new probe was added, and it had already drifted from the catalogue (missing types like 1120, 1122, 1123, 1200, 3010, 3020).I added a post-processing script (
resources/postprocess_neuropixels_probe_features.py) that derives two mappings from the catalogue JSON after each weekly ProbeTable sync:z_imro_format_type_to_imro_format(format type code to field schema) andz_imro_format_type_to_part_number(format type code to canonical part number for geometry construction)._parse_imro_stringnow takes just the IMRO string, extracts the format type from the header, and looks up the schema directly.read_imrouses the part number mapping instead of the hand-maintained dict.Something to discuss: while working on this, I noticed that some format type codes in the old dict (NHP types 1015, 1021, 1022, 1031, 1032 and NP2.x variants 2004, 2014) are not listed in any ProbeTable
val_defentry. My recollection is that the old dict had them because it was originally designed to map part numbers to types (where one entry per part number made sense) and was later repurposed in reverse to resolve types back to part numbers. I have left them in a small fallback dict for backwards compatibility, but I think we should remove them after a release or two if nobody reports a breakage. What do you think?