-
Notifications
You must be signed in to change notification settings - Fork 260
Remove auto_cast_uint, cast_unsigned, and modify fix_dtype #3982
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -405,6 +405,9 @@ def fix_dtype(recording, dtype): | |
|
|
||
| # if uint --> force int | ||
| if dtype.kind == "u": | ||
| dtype = np.dtype(dtype.str.replace("u", "i")) | ||
| raise ValueError( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would consider making this just a little clearer. I think users would want to know why they aren't supported. So I would recommend adding an additional sentence that says: "unsigned types are not supported due to **" I'm just trying to think about what to say? inconsistent behavior in the library? Or to flip it and say that ints and floats perform better. The issue we are referencing has someone who is saying "I want to use unsigned" so I think we should at least say even in short form why they should not. I agree that maybe some sort of documentation we can point to that explains this or GitHub issue referenced would at least let the users know the real reason why they can't. Especially since this is a change in behavior.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure. We can say that scopy functions don't work well with unsigned ;)
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @zm711 better?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missed this but yeah that looks good to me :) |
||
| "Unsigned types are not supported. You can use " | ||
| "`spikeinterface.preprocessing.unsigned_to_signed` to convert the recording to a signed type." | ||
| ) | ||
|
|
||
| return dtype | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like a clear enough deprecation to me :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant in
get_traces.