Let get_streams work from the path alone on Neuralynx, Biocam and Blackrock - #4727
Open
h-mayorquin wants to merge 2 commits into
Open
Let get_streams work from the path alone on Neuralynx, Biocam and Blackrock#4727h-mayorquin wants to merge 2 commits into
get_streams work from the path alone on Neuralynx, Biocam and Blackrock#4727h-mayorquin wants to merge 2 commits into
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.
get_streamsandget_num_blockshand their arguments straight tocls.map_to_neo_kwargs, and callers pass only the path, since that is all most extractors need. Neuralynx and Biocam declare extra parameters there without defaults, soget_neo_streams("neuralynx", folder_path)raisesTypeError: map_to_neo_kwargs() missing 2 required positional argumentsand Biocam does the same forfill_gaps_strategy. The error comes from our own signature rather than from neo, which has good defaults for all of these, and it fires before the reader is ever constructed, which leaves the function the how-to guides recommend for listing streams unusable on those two formats. Blackrock was unreachable from the other direction, sincegap_tolerance_msnever enteredmap_to_neo_kwargsat all and was injected intoneo_kwargsinside__init__afterwards.I gave the Neuralynx and Biocam parameters the same defaults their constructors already pass, so discovery opens the file the way the extractor will rather than the way neo would, and moved Blackrock's
gap_tolerance_msintomap_to_neo_kwargsso that it travels with the path. This has broken once per neo argument added since 2024, each time in a pull request that added the argument to the constructor without noticing the second caller, so the guard belongs inRecordingCommonTestSuiterather than in a test naming the three formats we happened to look at. It now callsget_streamsfor every entity, which covers each neo format we have data for and any format added later. I also gaveNeuralynxSortingExtractortheexclude_filenamethat its recording counterpart has had since 2024, keeping neo's own default, since it reads through the sameNeuralynxRawIOand simply never got it.