Skip to content
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

Add NeuropixelsV2eBeta GUI #266

Closed
wants to merge 2 commits into from
Closed

Add NeuropixelsV2eBeta GUI #266

wants to merge 2 commits into from

Conversation

bparks13
Copy link
Member

@bparks13 bparks13 commented Aug 27, 2024

This PR adds GUIs for the NeuropixelsV2eBeta headstage. The added dialogs are functionally similar to the NeuropixelsV2e GUIs, but use the correct underlying type (i.e., *Beta types).

Closes #227
Fixes #202

- This is almost exactly identical to the NeuropixelsV2e GUI, due to the many similarities
- Adds dialogs at all levels (probe / device / headstage)
@bparks13 bparks13 added this to the 0.3.0 milestone Aug 27, 2024
@bparks13 bparks13 requested a review from jonnew August 27, 2024 18:31
@bparks13 bparks13 self-assigned this Aug 27, 2024
Copy link
Member

@jonnew jonnew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per our discussion in the meeting today, the feeling is that there must be a way to reuse the non-beta NP2.0 GUI elements instead of all this code reproduction. Perhaps by

  • Defining an interface and operating on the interface
  • Defining a common, intermediate base type for both headstages in the main library
  • Performing a hard cast instead of testing for types (probably not great)

@bparks13
Copy link
Member Author

@jonnew If I try to cast directly using var configureHeadstage = (ConfigureNeuropixelsV2eHeadstage)component;, where component is of type ConfigureNeuropixelsV2eBetaHeadstage, I get an exception.

image

I'll work on implementing a common interface that both headstages implement, then I can pass the interface to the existing dialog instead of copying all of this code.

For cleanliness, I'm going to convert this PR to a draft, and then close it once these new changes are implemented.

@bparks13
Copy link
Member Author

This has been implemented much more efficiently in #269. This PR is no longer needed.

@bparks13 bparks13 closed this Aug 28, 2024
@bparks13 bparks13 deleted the issue-227 branch August 28, 2024 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add NeuropixelsV2eBeta GUI ProbeConfigurationA & ProbeConfigurationB properties lack documentation
2 participants