Improve docstring of get_neuropixels_sample_shifts
#3620
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zm711
reviewed
Jan 14, 2025
| Neuropixels 2.0 probes have 16 cycles. | ||
| If None, the num_channels_per_adc is used. | ||
| If None, defaults to the value of num_channels_per_adc. |
There was a problem hiding this comment.
Suggested change
| If None, defaults to the value of num_channels_per_adc. | |
| If None, defaults to the value of `num_channels_per_adc`. |
:)
There was a problem hiding this comment.
Thanks, will change in the next commit.
| @@ -42,10 +50,11 @@ def get_neuropixels_sample_shifts(num_channels=384, num_channels_per_adc=12, num | |||
| np.arange(num_channels), 2 | |||
| ) | |||
|
|
|||
| sample_shifts = np.zeros_like(adc_indices) | |||
| sample_shifts = np.zeros_like(adc_indices, dtype=float) | |||
There was a problem hiding this comment.
remind me the benefit of using python float in the case of numpy functions? This just auto scales the dtype between float32 and float64? Could this create float16 that could overflow? I honestly and super rusty on this stuff :)
There was a problem hiding this comment.
It is an alias that will auto scale depending on the OS if I remember well. But now that I think about it, I don't want to introduce a change on behavior on this PR so I am removing this. Thanks for the catch.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I am working on improving the NWB annotation of this property and I took the time to improve the docstring of this function. I also added type hints.