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

Rename from ChannelConfiguration to ProbeGroup #321

Open
bparks13 opened this issue Oct 2, 2024 · 3 comments
Open

Rename from ChannelConfiguration to ProbeGroup #321

bparks13 opened this issue Oct 2, 2024 · 3 comments
Assignees
Milestone

Comments

@bparks13
Copy link
Member

bparks13 commented Oct 2, 2024

In many places, the name ChannelConfiguration was used for probe groups prior to any channel configurations were created. However, this can be a cause for confusion, and should be renamed for clarity.

An example of an existing place this occurs is below:

https://github.com/open-ephys/onix-bonsai-onix1/blob/de7bb7ef19d2c8c59d78c3a504a701c6ee53bd3a/OpenEphys.Onix1/NeuropixelsV2QuadShankProbeConfiguration.cs#L213

@bparks13 bparks13 self-assigned this Oct 2, 2024
@jonnew
Copy link
Member

jonnew commented Oct 3, 2024

ProbeGroup is not as clear to me as ChannelConfiguration. If the later is somehow incorrect, then lets find a happy medium that describes what the type does. ProbeGroup just doesn't give much information IMO.

@bparks13
Copy link
Member Author

bparks13 commented Oct 7, 2024

@jonnew The issue in my opinion is that ProbeGroup actually describes the object itself; this is how ProbeInterface labels the object, and what is currently called ChannelConfiguration is actually a probe group. The secondary issue is that we are calling the device level configuration classes *ProbeConfiguration to encapsulate all aspects of configuration, including the probe group.

For me it is confusing to access ProbeConfiguration.ChannelConfiguration when I really want to access the probe group. I do see what you mean about ChannelConfiguration being more descriptive, but I think it is actually taking away from what the object really is; a probe group contains the channel configuration information as well as the channel locations (even inactive channels), the shape of the probe shanks, as well as metadata associated with the probes.

Let's keep discussing this before making any changes.

@jonnew
Copy link
Member

jonnew commented Oct 10, 2024

I think this is reasonable. Also, maintaining the same naming conventions as the original specifications and API implementation is important for a lot of reasons. So go ahead do this with that in mind.

@jonnew jonnew added this to the 0.5.0 milestone Oct 15, 2024
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

No branches or pull requests

2 participants