search

catalystneuro / neuroconv

Create NWB files by converting and combining neural data in proprietary formats and adding essential metadata.
https://neuroconv.readthedocs.io
BSD 3-Clause "New" or "Revised" License
51 stars 21 forks source link

[Feature]: change metadata lists to dictionaries #269

Open bendichter opened 1 year ago

bendichter commented 1 year ago

What would you like to see added to NeuroConv?

This came from a conversation with @CodyCBakerPhD. Some of the metadata, e.g. Devices is added as a list. I'd like to change this to a dictionary. It would look something like this:

Devices:
  dev1:
    name: "device"
    description: "my device"

I would call dev1 a "tag". It identifies the device but is not any of the device metadata. Then in run_conversion you should be able to specify the device tag you want to use interface.run_conversion(..., device_tag="dev1") This would allow multiple interfaces to easily share the same device or use different devices. If device_tag is not supplied and there is only one device, that device is used. If there is more than one device, it throws an error which informatively lists the possible values for device_tag

Thoughts? How hard would this be to implement? Are there down-sides other than implementation effort and backwards compatibility issues?

Is your feature request related to a problem?

No response

Do you have any interest in helping implement the feature?

Yes.

Code of Conduct

bendichter commented 1 year ago

An alternative would be to make the names the keys of the dictionary. The advantages would be that the yaml file would be more compact and the uniqueness of the name would be enforced.

h-mayorquin commented 1 year ago

This is great! I did not see this before. +1 for making the names the keys of the dictionary.

CodyCBakerPhD commented 2 months ago

I think this has mostly shifted over time

@h-mayorquin Maybe you want to do one last glance over the repo to see if you spot any cases and if so document them here? Or otherwise close

h-mayorquin commented 2 months ago

Let me take a brief look, identify the ones that are missing and once we open a new issue we can close this.

h-mayorquin commented 2 months ago

OK, so at least in ecephys. Both the devices and the electrode groups are passed as dictionaries. In Ophys, devices and imaging planes and even the series are passed as lists.

So, I think it actually has shifted very little. Better to keep this open.

h-mayorquin commented 2 months ago

The most glaring problem is the fact that ImageSeries are access by index instead of by key (like the ecephys do) but this is very hard to fix without breaking current working (and past) conversions. The ideal point is to change this when we integrate the microscopy extension (which hopefully should be .... soon).