Probe refactoring with wiring#4548
Conversation
|
From the discussion today with @samuelgarcia
|
| if "probegroup" in dic: | ||
| from probeinterface import ProbeGroup | ||
|
|
||
| probegroup = ProbeGroup.from_dict(dic["probegroup"]) | ||
| # The `wiring` per-channel property was restored above by the standard | ||
| # property-load loop; we just attach the probegroup object. | ||
| extractor._probegroup = probegroup | ||
| elif "contact_vector" in dic.get("properties", {}): | ||
| _restore_probegroup_from_legacy_contact_vector(extractor) |
There was a problem hiding this comment.
this should not be in base, since it's specific to the BaseRecordingSnippets
| return extractor | ||
|
|
||
|
|
||
| def _restore_probegroup_from_legacy_contact_vector(extractor) -> None: |
| key_to_int = {k: i for i, k in enumerate(unique_keys)} | ||
| groups = np.array([key_to_int[k] for k in group_keys_per_channel], dtype="int64") | ||
|
|
||
| target.set_property("location", locations) |
There was a problem hiding this comment.
As part of this effort, I think we should get rid of the location property and only keep back-compatibility to re-intantiate a probe object
| # when a probe is attached, derive groups on the fly from the `wiring` | ||
| # property + probegroup state (probe_id + shank_ids + contact_sides) |
There was a problem hiding this comment.
Why do we need to do this on the fly? I think that the current implementation of setting the group property based on some group_mode (auto, by_probe, by_side, etc.) was explicit and clear. No?
| """ | ||
|
|
||
| _main_properties = ["group", "location", "gain_to_uV", "offset_to_uV"] | ||
| _main_properties = ["group", "location", "wiring", "gain_to_uV", "offset_to_uV"] |
There was a problem hiding this comment.
| _main_properties = ["group", "location", "wiring", "gain_to_uV", "offset_to_uV"] | |
| _main_properties = ["group", "wiring", "gain_to_uV", "offset_to_uV"] |
|
Lets discuss this more. For me if we attach a probegroup with 4 probes to a recording and then we split it, I think we expect that each new sub recording has only one probe. I also think that the get_probe must give a sorted version of the probe that match the channel order. In short I am OK to have a property for the mapiing (probe_index, contact_id) <-> channel_id but In short we have 2 choice:
Since a recording can easily acces all parents, and so there related probegroup. I do not see any advantage of going to 1.. I really refer 2. |
|
@samuelgarcia if I understood correctly you are Ok with keeping the internal representation as a The second point that you mentioned in our meeting today is the background compatibility but we did not finish that line of discussion because I needed to go. Can you expand and sketch a test of what I should do to get the error you were mentioning? |
3 participants
More ambitious, more complete, greater, faster, stronger.
See: #4553