Add additional `unsigned_to_signed` doc #4076

zm711 · 2025-07-19T17:44:04Z

As discussed at our maintenance meeting here is the first draft of our unsigned_to_signed doc for people converting to 0.103.0 and running into the value error.

@chrishalcrow @h-mayorquin would love for your opinions on doc side.

@alejoe91, please just push directly to the PR with your fixes for bit depth.

Link to docs: https://spikeinterface--4076.org.readthedocs.build/en/4076/how_to/unsigned_to_signed.html

doc/how_to/unsigned_to_signed.rst

chrishalcrow · 2025-07-19T21:42:50Z

doc/how_to/unsigned_to_signed.rst

+    print(recording)
+    # use method on the Recording object
+    print(recording.get_dtype())
+


Would be good to mention what you'd expect here. E.g. "If this returns a dtype starting with a u, such as uint16, the recording if using unsigned int. While if this returns ... "

Do you think I should just write up a parsed-literal block or you think it is sufficient to just say uint16?

Up to you. I think sufficient just to say: the possible things it can return are pretty simple.

doc/how_to/unsigned_to_signed.rst

chrishalcrow · 2025-07-19T21:51:05Z

Very clear and easy to understand - just some minor typos, and one request. Thanks @zm711 !!

Co-authored-by: Chris Halcrow <[email protected]>

h-mayorquin

This looks good to me. If we can, I think it will be better to be specific about the formats that are affected by this. This will be useful for users so they can see if they are affected by this and/or can disregard this information otherwise.

doc/how_to/unsigned_to_signed.rst

h-mayorquin · 2025-07-21T17:59:50Z

doc/how_to/unsigned_to_signed.rst

+
+As of version 0.103.0 SpikeInterface has changed one of its defaults for interacting with
+:code:`Recording` objects. We no longer autocast unsigned dtypes to signed implicitly. This
+means that some users of SpikeInterface will need to add one additional line of code to their scripts


Maybe mention the formats here that suffer from it?

I think this is the perfect being the enemy of good. The only way I can think of doing this is:

looking through the code in neo for each file format to see how it is stored

testing this myself to see how each is stored

both of these are too time-intensive for me. Are you interested in making this list for us and then I'll add it in?

No. I don't have time either.

We don't have the resources to be exhaustive but do we know at least or or two examples. Didn't this cause troubles for you and that's how you noticed?

Yes it is in the example. I know Intan does this. But that's the only one I know for sure off the top of my head. (I could do a e.g. Intan) but I also didn't want to preference one system.

I think it is fine to add only one to have at least is one example. We add more if we discover them to be more. I guess we can just do a search for uint16 in neo but for this PR at least let's have an example.

Let me look for at least a second example if that exists.

Maxwell:

https://github.com/h-mayorquin/python-neo/blob/2240a29c4366fb3ed5dacadb54837004651ee35e/neo/rawio/maxwellrawio.py#L184-L192

Biocam as well it seems:

https://github.com/h-mayorquin/python-neo/blob/2240a29c4366fb3ed5dacadb54837004651ee35e/neo/rawio/biocamrawio.py#L87-L100

Thansk guys for looking this up. TO my knowledge, it's indeed Intan, Maxwell, and Biocam. Let's start adding these and add more if they pop up

I can add the list in another PR if you want @zm711. You have already done a lot with this : )

h-mayorquin · 2025-07-21T18:01:25Z

doc/how_to/unsigned_to_signed.rst

+-----------------
+
+For those that want a deeper understanding of dtypes `NumPy provides a great explanation <https://numpy.org/doc/stable/reference/arrays.dtypes.html>`_.
+For our purposes it is important to know that many pieces of recording equipment opt to store their electrophysiological data as unsigned integers,


"Many pieces" maybe being specific here would help if you know the formats that do this.

"acquisition systems"? is that better? I don't necessarily want to go into depth here unless we do end up doing an exhaustive list. So I guess answering your statement above will determine what we do here.

Yes, same as above. Let's give some examples.

h-mayorquin · 2025-07-21T18:02:10Z

doc/how_to/unsigned_to_signed.rst

+solution required is to convernt unsigned integers into signed integers. Previously we did this under the hood, automatically for users that had
+a :code:`Recording` object with an unsigned dtype.
+
+We decided, however, that implicitly performing this action was not the best course of action. So from version 0.103.0, users will now explicitly


If we remember it here we can give the reasoning. Do we remember?

I don't, other than implicit bad: explicit good. So you get a better provenance because previously we were doing a secret step that wasn't being recorded in provenance files necessarily (I think) haven't checked. Maybe worth discussion or @alejoe91 can add during his review.

Yes, if that is our reasoning I think is good to just say that. "We decide however to stop doing implicit logic with side effects here ... " or something along those lines.

Okay, maybe I can text Alessio tomorrow morning for a chat to see if he wants me to add. I could imagine explicit better, + better control of bit_depth (but I don't remember how the old function worked).

Yes, it is just that it is even bad (even for us) if we say we decided and we don't add a rationale. Beats the purpose of documenting this. I rather remove the first sentence instead of giving a statement of what we did if we don't give a rationale.

alejoe91 · 2025-07-22T07:43:38Z

doc/how_to/unsigned_to_signed.rst

+
+For those that want a deeper understanding of dtypes `NumPy provides a great explanation <https://numpy.org/doc/stable/reference/arrays.dtypes.html>`_.
+For our purposes it is important to know that many pieces of recording equipment opt to store their electrophysiological data as unsigned integers,
+which provides the benefit of reducing the necessary file size. In order to convert to real units these file formats only need to store a :code:`gain`


I don't think this is true, since unit16 and int16 has the same size (16 bit). The difference is just in the range of values they represent

That's basically what I was trying to get at. Because you can store a greater range of values you can use a smaller size unit but since we have to convert anyway to signed (or float) at some point this point is moot. I agree poorly written for what I was trying to get at and maybe not pertinent to the ephys case. We can rewrite!

alejoe91 · 2025-07-22T08:05:01Z

@zm711 Weirdly I cannot push to this branch:

To github.com:zm711/spikeinterface.git
 ! [remote rejected]     unsigned -> unsigned (permission denied)
error: failed to push some refs to 'github.com:zm711/spikeinterface.git'

I'll make a PR from my fork instead :)

EDIT: here it is zm711#3

zm711 · 2025-07-22T09:11:16Z

@zm711 Weirdly I cannot push to this branch:
To github.com:zm711/spikeinterface.git
 ! [remote rejected]     unsigned -> unsigned (permission denied)
error: failed to push some refs to 'github.com:zm711/spikeinterface.git'
I'll make a PR from my fork instead :)

EDIT: here it is zm711#3

I get that sometimes too. Usually if I just retry it works--I think it is actually a GitHub temp failure. I just double checked and the allow maintainers is definitely checked okay :)

I'll check out the PR!

Changes from Alessio

h-mayorquin

Ah you already added my requests, thanks! LGTM.

h-mayorquin · 2025-07-22T14:57:58Z

doc/how_to/unsigned_to_signed.rst

+
+As of version 0.103.0 SpikeInterface has changed one of its defaults for interacting with
+:code:`Recording` objects. We no longer autocast unsigned dtypes to signed implicitly. This
+means that some users of SpikeInterface will need to add one additional line of code to their scripts


I can add the list in another PR if you want @zm711. You have already done a lot with this : )

h-mayorquin · 2025-07-22T14:58:41Z

doc/how_to/unsigned_to_signed.rst

+solution required is to convernt unsigned integers into signed integers. Previously we did this under the hood, automatically for users that had
+a :code:`Recording` object with an unsigned dtype.
+
+We decided, however, that implicitly performing this action was not the best course of action, since:


Great. Thanks.

first draft of unsigned_to_signed doc

99e6d50

zm711 added the documentation Improvements or additions to documentation label Jul 19, 2025

chrishalcrow reviewed Jul 19, 2025

View reviewed changes

doc/how_to/unsigned_to_signed.rst Outdated Show resolved Hide resolved

chrishalcrow reviewed Jul 19, 2025

View reviewed changes

doc/how_to/unsigned_to_signed.rst Outdated Show resolved Hide resolved

chrishalcrow reviewed Jul 19, 2025

View reviewed changes

doc/how_to/unsigned_to_signed.rst Outdated Show resolved Hide resolved

chrishalcrow reviewed Jul 19, 2025

View reviewed changes

doc/how_to/unsigned_to_signed.rst Outdated Show resolved Hide resolved

zm711 and others added 3 commits July 20, 2025 18:44

Chris' fixes

76755ee

Co-authored-by: Chris Halcrow <[email protected]>

more Chris requests.

f5f0bfa

fix rendering typo

56abb70

alejoe91 added this to the 0.103.0 milestone Jul 21, 2025

h-mayorquin reviewed Jul 21, 2025

View reviewed changes

alejoe91 reviewed Jul 22, 2025

View reviewed changes

Some clarifications and bit depth

ca4db72

zm711 added 2 commits July 22, 2025 05:13

Merge pull request #3 from alejoe91/unsigned

9eee046

Changes from Alessio

Merge branch 'main' into unisgned

6ea0c3c

alejoe91 approved these changes Jul 22, 2025

View reviewed changes

h-mayorquin approved these changes Jul 22, 2025

View reviewed changes

h-mayorquin merged commit fb9ddbb into SpikeInterface:main Jul 22, 2025
15 checks passed

zm711 deleted the unisgned branch July 28, 2025 12:19

Add additional unsigned_to_signed doc #4076

Add additional unsigned_to_signed doc #4076

Uh oh!

Conversation

zm711 commented Jul 19, 2025 • edited by chrishalcrow Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chrishalcrow commented Jul 19, 2025

Uh oh!

h-mayorquin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alejoe91 Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alejoe91 commented Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zm711 commented Jul 22, 2025

Uh oh!

h-mayorquin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Add additional `unsigned_to_signed` doc #4076

Add additional `unsigned_to_signed` doc #4076

zm711 commented Jul 19, 2025 •

edited by chrishalcrow

Loading

alejoe91 Jul 22, 2025 •

edited

Loading

alejoe91 commented Jul 22, 2025 •

edited

Loading