NeurodataWithoutBorders / matnwb

A Matlab interface for reading and writing NWB files
BSD 2-Clause "Simplified" License
48 stars 32 forks source link

[Feature]: Use new name=val pattern #570

Open bendichter opened 5 months ago

bendichter commented 5 months ago

What would you like to see added to MatNWB?

MATLAB now allows you to have the syntax func(a=1, b=2), which is nicer and would be more similar to PyNWB. It would be nice to look at converting the library to use this syntax.

Is your feature request related to a problem?

No response

What solution would you like?

see above

Do you have any interest in helping implement the feature?

Yes.

Code of Conduct

lawrence-mbf commented 5 months ago

Note that this syntax is only available in MATLAB R2021a which may impact users stuck or unwilling to upgrade to newer versions.

bendichter commented 5 months ago

Yes, I have seen some applications where you can keep the old format as well, e.g. with plotting: https://www.mathworks.com/help/matlab/matlab_prog/namevalue-in-function-calls.html I'm not sure how we would implement this ourselves

ehennestad commented 4 months ago

It is already possible to use this syntax without changing anything in the library.

The question is, should tutorials be updated to use this newer syntax? And then the problem is, as @lawrence-mbf notes, that people using older versions would not be able to run the tutorials

rly commented 4 months ago

PyNWB supports all currently alive/active versions of Python. Each version lasts five years. The documentation does not use features specific to only a subset of those versions.

I am in favor of updating the matnwb tutorials to use recently released MATLAB features that improve quality of life for the vast majority of users. Users running older versions can still use matnwb but would need to adjust the tutorial code. We could add a note about that in each tutorial, but this is pretty unfriendly to novices. I hope that this would impact relatively few users.

What should the time (or version) cutoff be? In my limited experience, when scientists start using a particular version of MATLAB for their data, they will not upgrade until the analysis is done, just in case the behavior of their code changes in later versions. Some people use older versions because their lab code doesn't run on newer versions. But they could also install a recent version of MATLAB to use matnwb without having to modify the tutorials. Three years sounds reasonable to me (which means I think we should update the tutorials to use the name=val pattern introduced in 2021a). Four or five years would be extra safe.

rly commented 4 months ago

We could also just try it and roll it back if there are significant complaints.

ehennestad commented 3 days ago

There is a special constructor mode for NWB types where values can be added to a set contained in the property of an object using name-value syntax in the class constructor. For example: position = types.core.Position('SpatialSeries', spatial_series_ts);

In this case, 'SpatialSeries' is not a property of the types.core.Position class, instead the spatial_series_ts will be added as a named value to the spatialseries property of types.core.Position, where spatialseries is a types.untyped.Set and 'SpatialSeries' is used as a name.

For this mode, the name=value syntax does not work. I therefore think it might be confusing to introduce the name=value syntax, as there will be cases where it will not work, and it is a quite technical explanation that I think users should not have to think about.

In my opinion, the best way forward would be to get rid of support for this special constructor mode.

A few reasons for this: