Deal with unsigned int to int conversion #1707
Conversation
alejoe91
added
core
|
Cool! This one would be mildly surprising to me to me since the behavior is different from |
There was a problem hiding this comment.
I have not seen the other issue but I think that astype preprocessing should behave in exactly the same way as numpy.astype. Just because that is the behavior that our users will probably expect. Is this the case?
Edit: after thinking about it I am less sure. I will need to think.
|
|
||
|
|
||
| def test_astype(): | ||
| traces = (np.random.rand(10000, 4) * 100).astype("float32") |
There was a problem hiding this comment.
I think these tests will work better with a user defined traces
Just write something like [1.0 ,3.0, 5.0, ...] or at least let's make them non-deterministic.
There was a problem hiding this comment.
I think it's ok to leave randomness here. It should work in any case since it's only casting the dtypes
There was a problem hiding this comment.
I think it is unlikely to create a problem here but I also don't see why the randomness is required. If it is meant to work as a test then it should be reproducible so you can debug it if it fails. But here, the failure is unlikely to come from randonmess so not a big concern ...
Anyway, not a big deal:
"A foolish consistency is the hobgoblin of little minds"
alejoe91
changed the title
|
@cwindolf @h-mayorquin made a new preprocessor Ready to review! |
src/spikeinterface/preprocessing/tests/test_unsigned_to_signed.py
Outdated
Show resolved
Hide resolved
|
I think we could have better implementation using numpy view. No one gave time to give feedback. |
|
@samuelgarcia it's been open for 3 weeks! |
|
Wrote some thoughts on #1748 |
Fixes #1702 so you can do the following for a recording of typoe unsigned:
Internally, it takes care of upcasting and removing the
2**(nbits-1)offset before adding the sign (to avoid overflow errors).Also added some tests ;)