matnwb icon indicating copy to clipboard operation
matnwb copied to clipboard

[Feature]: Use new name=val pattern

Open bendichter opened this issue 1 year ago • 6 comments

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

bendichter avatar Jun 13 '24 16:06 bendichter

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

lawrence-mbf avatar Jun 24 '24 15:06 lawrence-mbf

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

bendichter avatar Jun 24 '24 17:06 bendichter

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

ehennestad avatar Jul 06 '24 20:07 ehennestad

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 avatar Jul 13 '24 14:07 rly

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

rly avatar Jul 13 '24 14:07 rly

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:

  • This mode prevents use of arguments blocks. arguments blocks unlocks autocompletion of name-value pairs and in general makes the code cleaner and more maintainable.
  • Passing name-value pairs to the constructor which is then added to a property with dictionary-like behavior behind the scenes is opaque and can easily cause confusion.
  • It is also not standard in MATLAB to pass name-value pairs to a class constructor where the name does not correspond to a property or options. When comparing the expression above with the class documentation, it is unclear to a user how an "arbitrary" name can be used as an input.
  • It deviates from how it is done in pynwb.

ehennestad avatar Nov 21 '24 20:11 ehennestad