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

[Feature]: Change implementation of containers types to simplify API #625

Open
2 tasks done
ehennestad opened this issue Nov 16, 2024 · 1 comment
Open
2 tasks done
Labels
category: proposal discussion of proposed enhancements or new features priority: low alternative solution already working and/or relevant to only specific user(s) topic: matnwb-api related to improving the matnwb api

Comments

@ehennestad
Copy link
Collaborator

ehennestad commented Nov 16, 2024

What would you like to see added to MatNWB?

Many NWB types are containers for other types, with two examples being ProcessingModule and Fluorescence.

The syntax for setting/getting data for these containers is cumbersome:

ophys_module = types.core.ProcessingModule('description', 'contains optical physiology data')

ophys_module.nwbdatainterface.set('ImageSegmentation', img_seg);

and

fluorescence = types.core.Fluorescence();

fluorescence.roiresponseseries.set('RoiResponseSeries', roi_response_series);
ophys_module.nwbdatainterface.set('Fluorescence', fluorescence);

A simpler way of doing this would be:

ophys_module = types.core.ProcessingModule('description', 'contains optical physiology data')

ophys_module('ImageSegmentation') = img_seg;

and

fluorescence = types.core.Fluorescence();

fluorescence('RoiResponseSeries') = roi_response_series;
ophys_module('Fluorescence') = fluorescence;

A similar suggestion was raised in pynwb (issue #1993) and it would be great to mirror this on the matnwb side.

Is your feature request related to a problem?

No response

What solution would you like?

Redefine container classes to have dictionary/Set like behavior, while also restricting the types it is allowed to hold

Do you have any interest in helping implement the feature?

Yes.

Code of Conduct

@ehennestad ehennestad added priority: low alternative solution already working and/or relevant to only specific user(s) category: proposal discussion of proposed enhancements or new features topic: matnwb-api related to improving the matnwb api labels Nov 16, 2024
@bendichter
Copy link
Contributor

bendichter commented Jan 27, 2025

@ehennestad this sounds like a nice change, as long as we maintain backwards compatibility and have good tests for both the old and the new mode. But note that this only works when a container has a single Set object, so we'll need to check for that and revert to only supporting the old style if there are multiple.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: proposal discussion of proposed enhancements or new features priority: low alternative solution already working and/or relevant to only specific user(s) topic: matnwb-api related to improving the matnwb api
Projects
None yet
Development

No branches or pull requests

2 participants