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

Automagically load necessary Python modules for extensions registered in nwb-extensions #1143

Closed
yarikoptic opened this issue Jan 17, 2020 · 2 comments · Fixed by #1748
Closed
Labels
category: enhancement improvements of code or code behavior

Comments

@yarikoptic
Copy link
Contributor

yarikoptic commented Jan 17, 2020

It might be that it is already intended, but found no issue yet, thus decided to be the one to "report" since I found no mentioning of nwb-extensions among issues or in the code.

ATM, NWBHDF5IO(...).read() would fail with an exception like 'AIBS_ecephys' not a namespace" leaving user baffled on "what have I done wrong?" or "how do I fix that?". If registry (I am still not sure if it captures all necessary information ATM) had provided listing of those "name spaces" and (Python) modules which need to be installed, then PyNWB would automagically import allensdk.brain_observatory.ecephys.nwb while .read()ing that file. And if import fails, it would provide an error message like

The file ... contains data of AIBS_ecephys neural data type, to load which PyNWB requires allensdk (>= X.Y.Z) to be installed.

Without such functionality, IMHO it would be pretty much impossible to develop generic PyNWB-based tools otherwise agnostic of underlying neural data types.

edit: in dandi/dandi-cli#36 I for now provide a prototypical workaround only for AIBS_ecephys for now

@yarikoptic yarikoptic added the category: enhancement improvements of code or code behavior label Jan 17, 2020
yarikoptic added a commit to dandi/dandi-cli that referenced this issue Jan 17, 2020
…ing exception with AIBS_ecephys listed missing

Ultimate solution should IMHO be implemented within PyNWB itself.
See NeurodataWithoutBorders/pynwb#1143 feature request.
Also related - dedicated exception (NeurodataWithoutBorders/pynwb#1144)

With this change we should be able to deal with neuropixels files from Allen
@bendichter
Copy link
Contributor

@yarikoptic By way of background, one solution we are developing for this is the load_namespaces arg of NWBHDF5IO. This reads cached schemas and automatically tries to generate the API for any missing namespaces. This works well in simple cases, but it won't allow users to leverage any custom APIs they have developed. So how would we do that?

The registry should provide information about how to install the necessary extension, but I really don't want to pip install anything without the user's permission. We could:

  1. Provide an error message that uses information from the registry to tell the user exactly how to install and import the necessary extension
  2. Provide a user-interaction feature where we ask the user if they would like us to install the necessary packages for them

@oruebel
Copy link
Contributor

oruebel commented Nov 3, 2020

'AIBS_ecephys' not a namespace"

I think for this specific error we should provide a few details on how to potentially resolve it, i.e., either use load_namespaces and or install the appropriate extensions.

Provide an error message that uses information from the registry to tell the user exactly how to install and import the necessary extension

I think the the first step is to actually use information from the file. I.e., we could check if an appropriate namespace is in the file and indicate this to user as part of the error message.

For the second part of checking the registry, I think rather than making this as part of the error checking, this should be a separate utility to allow users to search the registry based on information from the file (e.g,. look for names of namespaces or possibly even neurodata_types). Since this will a) require online access and b) potentially take some time, I think this should be a separate step. However, this is a step we could mention in the error message to indicate that they may want to use additional functionality to locate and install extensions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: enhancement improvements of code or code behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants