-
Notifications
You must be signed in to change notification settings - Fork 39
Deprecate brainbox.processing.bincount2D
#622
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
Conversation
k1o0
left a comment
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.
Looks good. Just change the requirements to a minor release. For the other rigs I've been raising both a DeprecationWarning and a logger warning in case users turn off the ibllib logger but that's overkill to be honest. We also usually include a print statement of the traceback because it's sometime very difficult to find the call to that function, like this:
ibllib/brainbox/behavior/wheel.py
Lines 88 to 89 in 381d648
| for line in traceback.format_stack(): | |
| print(line.strip()) |
It's also super annoying though (which may be a good thing). Personally I don't think it's particularly necessary.
Finally, Olivier adds something that breaks the test as a reminder to remove the deprecated function e.g. 2f6a13e. I just set a reminder in Slack once the branch has been merged for release.
|
New branch |
From the discussion in int-brain-lab/ibl-neuropixel#13 we are going to move
bincount2Dtoiblutil.This replaces
brainbox.processing.bincount2Dwith a call to the imported version fromiblutiland adds a warning.Also updates the
requirementstoiblutil1.6.1, which contains the replaced method. (int-brain-lab/iblutil#19)