matnwb package

[bahanonu]

trafficstars

Is there any plan to move the current matnwb packages into sub-packages of a matnwb package for a cleaner namespace?

For example, instead of calling types.core.OpticalChannel users could call matnwb.types.core.OpticalChannel and this would be true for all functions in the current top-level packages.

This would also reduce potential conflicts given the names of the current top-level packages—whereas currently, matnwb may have to be added or remove from the path each time a NWB read/write operation is performed to avoid this (e.g. if a parent function has types= ''; or similar command, issues could arise).

This would not be a minor change (since other packages and code depend on the current setup), but may be worth considering.

[lawrence-mbf]

No but your timing is pristine as I can say that the alternative table constructor in native MATLAB does break for precisely this namespace reason. Evidently, the script used for the internal tabular class uses a variable called types which interferes with regular MatNWB use, especially when allocating space using ObjectViews.

You are correct that this will be quite a bit of effort as not only does the code regularly reference other functions within itself, the generated class locations will all need to be changed, and there will definitely be breakage in the utility scripts and other third-party applications that utilize the generated classes and custom types.

[bendichter]

I like namespaces as much as the next Python programmer, but this seems like a lot.

[bahanonu]

@ln-vidrio Thanks for the heads up about table, that's a good example case.

It is possible depending on the timescale of implementation, users could be notified of the change (as MathWorks currently does) and that new functions would be put under the matnwb package (with several ways that would allow a gradual transition of existing functions to a matnwb package).

For existing users, if a matnwb package is made, many would likely just need to start functions/scripts with import matnwb.* if they want to use the current naming conventions.

Else can just rely on adding/removing matnwb from path as needed to avoid these issues.

[ehennestad]

I support this!

FYI:

Matlab provides an option to define an alias file, which supports backwards compatibility of names.

The matlab.alias.AliasFileManager class provides an API for creating and maintaining alias definitions. Aliases are not part of class definitions. Instead, alias definition files are stored in resources folders that are located in the same folder as the latest class file.

This would ensure that utility functions and third party applications would continue to work as is (for a transition period, lets say).

[ehennestad]

I gave this an attempt here: https://github.com/ehennestad/matnwb

All the packages are moved into a main matnwb package, and all the package prefixes I could find have been renamed accordingly.

I ran the generateCore again, and it generates types with correct new names.

A few known issues that need further testing/investigating:

• I did not investigate fully whether functions depending on the misc.getMatnwbDir / matnwb.misc.getMatnwbDir will still work as intended
• I have not run any tests
• I don't know if I have found all cases where a classname is built dynamically by joining package prefixes using e.g sprintf or other cases where only partial package names of a function or class is coded explicitly.
• Documentation is not updated

Note: I also found a few references to classes that appears to have moved from types.core to types.hdmf_common. See here: d11751b and here: 04fbecf

Final note: The alias.json will only work with classes but I assume most external software will work with the types classes

[lawrence-mbf]

I did not investigate fully whether functions depending on the misc.getMatnwbDir / matnwb.misc.getMatnwbDir will still work as intended

You would need to add another fileparts(matnwbDir) after line 9 but that should resolve it for all cases.

I have not run any tests

• tests.system.PyNWBIOTest line 39 should add +matnwb to the top level search.
• The nwbtest.m needs to search from the matnwb.tests package.
• ignoreFolders and ignorePaths need to prepend matnwb to their paths as well. I think everything else will end up being automated.

I don't know if I have found all cases where a classname is built dynamically by joining package prefixes using e.g sprintf or other cases where only partial package names of a function or class is coded explicitly.

This is only really done in file generation so if that is successful it might be fine though that will require running tests.

Looks awesome!

[ehennestad]

Thanks! I think I got everything sorted out for running the tests.

Very many of the tests fail with this error: Values for property 'file_create_date' are not equal The value appears to be exactly one month wrong. I did add breakpoints in the NWB file when the date is created and then it appears correct, so this is something I don't understand.

Another issue I have is that the python tests don't succeed I assume because I don't have pynwb installed.

[ehennestad]

I tracked down the date issue to this function io.timestamp2datetime

This code snippet illustrates the behavior:

DatetimeCorrect = datetime(0, 0, 0, 0, 0, 0, 0);
DatetimeIncorrect = datetime(0, 0, 0, 0, 0, 0, 0);

ymdStamp = '2024-02-02';

YmdToken = struct(...
    'Year', ymdStamp(1:4),  ...
    'Month', ymdStamp(6:7), ...
    'Day', ymdStamp(9:10) );

DatetimeCorrect.Day = str2double(YmdToken.Day);
DatetimeCorrect.Month = str2double(YmdToken.Month);
DatetimeCorrect.Year = str2double(YmdToken.Year);

DatetimeIncorrect.Year = str2double(YmdToken.Year);
DatetimeIncorrect.Month = str2double(YmdToken.Month);
DatetimeIncorrect.Day = str2double(YmdToken.Day);

Turns out setting the month before the day can flip the month value one step ahead. The default value for datetime(0, 0, 0, 0, 0, 0, 0) is 30-nov, and when setting the month to February, since February does not have 30 days the month and day is adjusted automatically to 01-Mar. So running tests in February is not a good idea :)

Setting the day before the month solves it

[lawrence-mbf]

Oh wow, I would not have expected that. Excellent job.

[ehennestad]

Yeh, thats one of the weirdest bugs ive encountered. Could you give me a brief explanation what is needed to run python tests? Or point me to documentation if that exists?

[lawrence-mbf]

If your python env is on your path, I think the tests should automatically use it.

If not, you can save an env.mat file with a pythonDir name in it that contains a full path to the directory containing your python binary. Running tests using nwbtest should then run the python compatibility tests.

For more information you can take a look at +matnwb/+tests/+system/PyNWBIOTest.m and its python test script equivalent +matnwb/+tests/+system/PyNWBIOTest.py

[vijayiyer05]

Turns out setting the month before the day can flip the month value one step ahead. The default value for datetime(0, 0, 0, 0, 0, 0, 0) is 30-nov, and when setting the month to February, since February does not have 30 days the month and day is adjusted automatically to 01-Mar. So running tests in February is not a good idea :)

Thanks for elucidating. I'm open to submitting this internally, but I think folks would stumble on why it's mixing & matching datetime w/ other types.

At a glance, I think the desired correct behavior might be achieved compactly in this way:

ymd = datetime(2024,2,2);
correctYear = year(ymd);
correctMonth = month(ymd);
correctDay = day(ymd);

[ehennestad]

Hi Vijay, I don't consider this as a bug in MATLAB! The issue was with an internal function in matnwb (io.timestamp2datetime) where a string is converted to a datetime value.

[bahanonu]

@rly @bendichter Following up from the NWB Workshop about converting matnwb so all functions are inside a package (similar to what @ehennestad has done at https://github.com/ehennestad/matnwb). Happy to help test.

[vijayiyer05]

@rly @bendichter Following up from the NWB Workshop about converting matnwb so all functions are inside a package (similar to what @ehennestad has done at https://github.com/ehennestad/matnwb). Happy to help test.

+1 on this point/issue. One recent/emergent advantage of root namespaces: they enable a toolbox's live script example(s) to be run straight from an Open in MATLAB Online link, as alluded at the end of this blog post. You can this workflow in action at the Brain Observatory Toolbox repository.

Additionally, it makes sense to me for NWB to lead way wrt use of root namespaces (such as matnwb or even, dare I say, nwb*). Because it would be excellent, for instance, for various algorithmic packages to have standardly (redundantly) named functions like 'toNWB' & 'fromNWB`.

(*) Personally I'm open to the possibility that three-letter acronym (TLA) namespaces can work in a future where compute environments are often configured to their specific domains (with containers or otherwise)

[ehennestad]

Following up from the NWB Workshop about converting matnwb so all functions are inside a package (similar to what @ehennestad has done at https://github.com/ehennestad/matnwb). Happy to help test.

I think what I have done so far was complete at the time of my latest commit, but I did not have the time to get into thorough testing on real-world nwb files.

I am also open to considering another namespace like nwb, although updating the tutorials was quite laborious, as there are a lot of links in there that needs to be updated manually and individually.