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]: avoid demanding users to generateCore and generateExtension manually #493

Open
2 tasks done
yarikoptic opened this issue Feb 15, 2023 · 6 comments
Open
2 tasks done
Assignees

Comments

@yarikoptic
Copy link

What would you like to see added to MatNWB?

I have initially expressed that opinion/desire in #491 (comment) where we are trying to ping point/resolve an issue forbidding opening an nwb file.

Documentation ATM in https://github.com/NeurodataWithoutBorders/matnwb#step-2-generate-the-api mandates users to call generateCore() to produce some extra files. On one hand it is ok as for instructions "how to install from source" which is the case in that description based on git clone of the repository.

But then

Is your feature request related to a problem?

Unable to open .nwb file(s) as encountered in #491 etc

What solution would you like?

For the "core" schema, I guess it would be possible to just ship "pre-generated" ones from this repo but it would not work for extensions generally. So, I think matnwb should, as needed, generate* whatever is needed to open a file and store those namespaces in some versioned (by schema, extension, and/or matnwb) cached location, e.g. ~/.cache/matnwb/ and not demand user to do anything "manually".

Do you have any interest in helping implement the feature?

No.

Code of Conduct

@lawrence-mbf lawrence-mbf self-assigned this Feb 21, 2023
@lawrence-mbf
Copy link
Collaborator

lawrence-mbf commented Feb 21, 2023

To clarify (from #491 discussion):

The scripts generateCore, generateExtension, and nwbRead should be clarified as to their uses:

  • For reading, nwbRead should be all you need. Class files are generated from the embedded spec by default without any extra input from the user.
  • For writing, it is advised to use generateCore and generateExtension to generate specific versions of the core schema as needed. You then export this environment with nwbExport.

Both of these use the same cache location <install location>/+types which is prone to being overwritten but this isn't a big deal unless you opt to use more advanced options (i.e. savedir and friends) with these scripts.

@lawrence-mbf
Copy link
Collaborator

lawrence-mbf commented Feb 21, 2023

Minor clarification on writes:

When you nwbExport (write) a NWB file, the entire cache location <install location/+types is exported as an embedded specification.

@yarikoptic
Copy link
Author

how do you envision "system-wide" installation of matnwb on a shared user system (e.g. to accompany an installation of matlab with its toolbox), where some users might want, some - write, versions of nwb files can vary?

  • should generateCore be ran by admins upon initial installation?
  • should they generateExtension for some known extensions right away?

@lawrence-mbf
Copy link
Collaborator

lawrence-mbf commented Feb 21, 2023

The main issue with system-wide installations would inevitably be MATLAB Path wrangling since you need some way to switch between different generated classes eventually.

If we must insist on a "system-wide" installation then I would advise preventing writes inside the matnwb/+types directory and insist on savedir everywhere. Then any generateCore or generateExtension calls must be handled by each user.

I think at some point the class files were automatically saved in the working directory but I forget why we changed that. Would definitely be preferable in this case.

@yarikoptic
Copy link
Author

  • For reading, nwbRead should be all you need. Class files are generated from the embedded spec by default without any extra input from the user.

ok, I have tried that on the first sample (listed failing first in https://github.com/dandi/dandisets-healthstatus) file and it failed to load:

(venv) (base) dandi@drogon:~/cronlib/dandisets-healthstatus$ matnwb=$PWD/matnwb-master ; git -C $matnwb describe --tags;  git -C $matnwb clean -dfx; MATLABPATH=$matnwb time matlab -nodesktop -batch "f='$f'; disp(util.getSchemaVersion(f)); nwb = nwbRead(f)" && echo "All good!" || echo "Exited with $?"; git -C $matnwb clean -dfx
v2.6.0.1-2-gda49922
2.0b
Unable to resolve the name 'types.core.TimeSeries'.

Error in io.parseGroup (line 85)
    parsed = eval([Type.typename '(kwargs{:})']);

Error in io.parseGroup (line 38)
    subg = io.parseGroup(filename, group, Blacklist);

Error in io.parseGroup (line 38)
    subg = io.parseGroup(filename, group, Blacklist);

Error in nwbRead (line 59)
nwb = io.parseGroup(filename, h5info(filename), Blacklist);

Command exited with non-zero status 1
14.28user 1.48system 1:27.68elapsed 17%CPU (0avgtext+0avgdata 955596maxresident)k
691032inputs+480outputs (732major+207636minor)pagefaults 0swaps
Exited with 1
(venv) (base) dandi@drogon:~/cronlib/dandisets-healthstatus$ echo $f
/mnt/backup/dandi/dandisets-healthstatus/dandisets-fuse/000003/sub-YutaMouse33/sub-YutaMouse33_ses-YutaMouse33-150218_behavior+ecephys.nwb

although seeing version 2.0b for that file gives me concern -- may be that outdated "beta" one is the reason?

meanwhile I have tried on f=/mnt/backup/dandi/dandisets-healthstatus/dandisets-fuse/000405/sub-s0/sub-s0_ses-Li-OF-052818.nwb and there it worked out fine.

Overall -- I think I will adjust the test to indeed just do plain nwbRead for now and see where we would get.

dandibot pushed a commit to dandi/dandisets-healthstatus that referenced this issue Apr 3, 2023
@lawrence-mbf
Copy link
Collaborator

Sorry for taking so long to respond.

although seeing version 2.0b for that file gives me concern -- may be that outdated "beta" one is the reason?

Yes, that was some schema history that MatNWB no longer supports.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants