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

Add is_header_parsed flag to quickly check if header information is available in BaseRawIO #1388

Merged
merged 1 commit into from
Jan 31, 2024

Conversation

h-mayorquin
Copy link
Contributor

@h-mayorquin h-mayorquin commented Jan 31, 2024

The BaseRawIO class has the method parse_header that extracts all the information from the formats and therefore is usually a computational heavy process:

https://github.com/h-mayorquin/python-neo/blob/e3742da276fe3bb9a1ea327489ebdbf06c026956/neo/rawio/baserawio.py#L166-L181

I propose adding a simple flag called is_header_parsed that is set to True after the header is parsed (i.e. parse_header is called). That way, users can check quickly if the header is alredy parsed without having to check for the contents of the header directly.

That is, we follow the mantra that "explicit is better than implicit" here from the Zen.

Use case in point:

From spikeinterface, we sometimes require one or two properties from the header before intializing the neo reader:

https://github.com/h-mayorquin/spikeinterface/blob/1b8ae2c0907aee49ca9cb44e19c78d9bbfadf6e2/src/spikeinterface/extractors/neoextractors/plexon.py#L62-L64

However, within the neo base extractor we then re-parse the whole file at initialization:
https://github.com/h-mayorquin/spikeinterface/blob/1b8ae2c0907aee49ca9cb44e19c78d9bbfadf6e2/src/spikeinterface/extractors/neoextractors/neobaseextractor.py#L40-L64

We could avoid this with an explicit flag such as the one proposed into the PR without having to make assumptions about what is on the header attribute.

@alejoe91 @weiglszonja

@h-mayorquin h-mayorquin changed the title Add is_header_parsed flag Add is_header_parsed flag to quickly check if header information is available in BaseRawIO Jan 31, 2024
@samuelgarcia
Copy link
Contributor

Good idea.
Thank you.

@weiglszonja
Copy link
Contributor

Sounds good to me, thanks @h-mayorquin!

@alejoe91 alejoe91 added this to the 0.13.0 milestone Jan 31, 2024
@samuelgarcia samuelgarcia merged commit 790d09c into NeuralEnsemble:master Jan 31, 2024
1 check passed
@h-mayorquin h-mayorquin deleted the add_is_parsed_flag branch February 1, 2024 11:22
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