-
Couldn't load subscription status.
- Fork 47
Add adc_group/adc_sample_order contact annotation
#368
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
mux_index contact annotation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #368 +/- ##
==========================================
+ Coverage 90.11% 90.13% +0.02%
==========================================
Files 12 12
Lines 2083 2088 +5
==========================================
+ Hits 1877 1882 +5
Misses 206 206 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I find mux_index and mux_channels confusing and somewhat clashing with other concepts, as "index" and "channel" are already overloaded terms in our ecosystem. Could we take this opportunity to improve the naming?
|
mux_index contact annotationadc_group/adc_sample_order contact annotation
| probe = _make_npx_probe_from_description( | ||
| pt_metadata, probe_part_number, elec_ids, shank_ids=shank_ids, mux_info=mux_info | ||
| ) | ||
| if chans_saved is not None: |
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.
| if chans_saved is not None: |
(this doesn't matter, but it's existence made me stare at the indentation for ages to check it was right...)
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.
not sure I understand!
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.
Oh I think I understand. We need this in both cases (if probe is none or not)
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 just meant that there was a repeated if chans_saved is not None, but you're right that it should have been moved down an indentation. Nice catch!
| probe = np_probe_info.get("probe") | ||
|
|
||
| if probe is None: | ||
| # check if subset of channels |
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 comment should move to the chans_saved code too
Fixes #367