Skip to content

Conversation

@yagui
Copy link
Contributor

@yagui yagui commented Sep 6, 2025

fix #1764

Copy link
Contributor

@zm711 zm711 left a comment

Choose a reason for hiding this comment

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

Nice catch. Looks good to me. Doc error is unrelated. I'm running tests now, but we should be able to merge after that is done!

@zm711 zm711 changed the title fix rawbinary reader RawBinaryIO: Fix hard-coded dtype during memmap generation Sep 8, 2025
@zm711 zm711 changed the title RawBinaryIO: Fix hard-coded dtype during memmap generation RawBinarySignalRawIO: Fix hard-coded dtype during memmap generation Sep 8, 2025
@zm711
Copy link
Contributor

zm711 commented Sep 8, 2025

Also once we get this merged would you like to be on our author list? If so could I get the name you want to display and any institutional affiliation that should go with your name.

Copy link
Contributor

@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.

LGTM.

@h-mayorquin
Copy link
Contributor

Maybe we should add to the todo to add a test that catches this and test other aspects of reading non-int16 binary data with this (say unsigned types) to see if we are not making any assumptions.

@zm711
Copy link
Contributor

zm711 commented Sep 8, 2025

I think there is a test like that in spikeinterface where we have caught issues like this before, but since spikeinterface manages its own binary file reading we do a have blind spot for these rawios.

@h-mayorquin h-mayorquin merged commit a1ac529 into NeuralEnsemble:master Sep 8, 2025
3 checks passed
@samuelgarcia
Copy link
Contributor

THanks.
Crazy that we catch this only now!!

@yagui yagui deleted the fix/raw-binary-read branch September 12, 2025 10:14
@zm711 zm711 added this to the 0.14.3 milestone Oct 4, 2025
@zm711
Copy link
Contributor

zm711 commented Oct 4, 2025

@yagui we are about to do a new release. Just let us know if you want to be included in the author list for this release. (We can also add you later if you want :) )

@yagui
Copy link
Contributor Author

yagui commented Oct 6, 2025

Thanks, this was just one line. If I ever do a bigger contribution I'll be honored to be in the list. Thank you for this awesome library.

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.

Reading a raw binary int16 file as uint16

4 participants