[Feature]: solve naming collisions in metadata

[bendichter]

What would you like to see added to NeuroConv?

NWBConverters currently have a usability problem due to naming collisions. For example, let's define a converter with two RecordingInterface classes, both of which define es_key = "ElectricalSeries" by default.

class NWBConverterChild(NWBConverter):
    data_interface_classes = dict(Interface1=MockRecordingInterface, Interface2=MockRecordingInterface)

converter = NWBConverterChild(dict(Interface1=dict(), Interface2=dict()))
converter.get_metadata_schema()["properties"]["Ecephys"]["properties"].keys()

dict_keys(['Device', 'ElectrodeGroup', 'Electrodes', 'definitions', 'ElectricalSeries'])

There are two interfaces, yet we end up with a single field ElectricalSeries, because their is a naming collision: both interfaces default to es_key="ElectricalSeries". So a user needs to know to override this with es_key="something_unique" in the source input for each interface, which is unintuitive and not a great user experience.

Solution
In a NWBConverter, we know that the interface names are unique, since they are all keys in the same dict, so if we prepend with those names, all of the "ElectricalSeries" fields in the metadata struct should also be unique.

Basically, I want:

>>> converter.get_metadata_schema()["properties"]["Ecephys"]["properties"].keys()
dict_keys(['Device', 'ElectrodeGroup', 'Electrodes', 'definitions', 'Interface1_ElectricalSeries', 'Interface2_ElectricalSeries'])

This comes up most often with ecephys, though ideally we would find a solution that would work for all modalities.

Thoughts, @CodyCBakerPhD ?

Is your feature request related to a problem?

No response

Do you have any interest in helping implement the feature?

Yes.

Code of Conduct

• I agree to follow this project's Code of Conduct
• Have you ensured this bug was not already reported?

[CodyCBakerPhD]

A name generator like in PyNWB (https://github.com/NeurodataWithoutBorders/pynwb/blob/dev/src/pynwb/testing/mock/utils.py) or like pytest factories use is usually the way to go however, some annoyances that can result from that are

1. the increment counter only resets with the kernel; if running in a notebook or the like, and errors or something else cause the code snippets for the interfaces to be re-called, the counter does not reset so you end up with a final converter that has es_keys of the like [Interface5_ElectricalSeries, Interface6_ElectricalSeries]

2. the numeric incrementor is not very semantic (instead of, e.g., some identified about the stream such as 'ap' or 'lf'), and can mismatch existing semantic conventions such as multi-probe SpikeGLX by generating something like

{
    'Interface1_ElectricalSeries': SpikeGLXRecordingInterface, # But this actually points to AP band of the imec2 not imec0
    ''Interface2_ElectricalSeries': SpikeGLXRecordingInterface, # points to LF band of imec1
}

which could also result in confusion

In general I'd suggest making the es_key some meaningful yet unique identified for that data stream; one possibility would be to literally just use the stream_name from SpikeInterface, but those aren't always super readable

Otherwise this is simply a 'learning curve' thing that can be countered via better documentation - also, this type of usability is of the type that is much easier to solve in the NWB GUIDE where the user can clearly communicate and interact with the settings in a more naturally intuitive way

[h-mayorquin]

I can take over this.