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

namespace checks #917

Closed
satra opened this issue Feb 18, 2022 · 3 comments · Fixed by #1149 · May be fixed by #1036
Closed

namespace checks #917

satra opened this issue Feb 18, 2022 · 3 comments · Fixed by #1149 · May be fixed by #1036
Assignees

Comments

@satra
Copy link
Member

satra commented Feb 18, 2022

this section of the pynwb CLI figures out all the cached namespaces in an NWB file and validates against them: https://github.com/NeurodataWithoutBorders/pynwb/blob/dev/src/pynwb/validate.py#L70

the validate function only validates against the CORE namespace. this has resulted in this error: dandi/helpdesk#43

there needs to be some refactoring in pynwb, but that's not going to fix dandi issues immediately unless we depend on the latest and greatest version. so perhaps some of the code could be imported into the dandi validate nwb function.

cc: @rly @oruebel

@oruebel
Copy link

oruebel commented Feb 18, 2022

In NeurodataWithoutBorders/pynwb#1432 I've extracted the code to determine the extension namespace from the file to a separate function pynwb.validate.get_chached_namespaces_to_validate so that the code can be reused to validate against cached namespaces when calling pynwb.validate directly from Python, rather than the command line. The code example below illustrates how you could fix this quickly in DANDI. Once PyNWB has been released you would then remove the get_chached_namespaces_to_validate from DANDI and simply replace it with the import from PyNWB from pynwb.validate import get_chached_namespaces_to_validate.

from pynwb import validate, NWBHDF5IO

def get_chached_namespaces_to_validate(path):
   """
   Determine the most specific namespace(s) (i.e., extensions) that are chached in the given
   NWB file that should be used for validation.

   :param path: Path for the NWB file
   :return: Tuple with:
     - List of strings with the most specific namespace(s) to use for validation.
     - BuildManager object for opening the file for validation
     - Dict with the full result from NWBHDF5IO.load_namespaces
   """
   # Importing locally here so we can easily switch to using pynwb.validate.get_chached_namespaces_to_validate
   # once it is part of the release
   from hdmf.spec import NamespaceCatalog
   from hdmf.build import BuildManager
   from pynwb.spec import NWBDatasetSpec, NWBGroupSpec, NWBNamespace
   from hdmf.build import TypeMap as TypeMap

   catalog = NamespaceCatalog(NWBGroupSpec, NWBDatasetSpec, NWBNamespace)
   ns_deps = NWBHDF5IO.load_namespaces(catalog, path)
   s = set(ns_deps.keys())  # determine which namespaces are the most
   for k in ns_deps:  # specific (i.e. extensions) and validate
      s -= ns_deps[k].keys()  # against those
   # TODO remove this workaround for issue https://github.com/NeurodataWithoutBorders/pynwb/issues/1357
   if 'hdmf-experimental' in s:
      s.remove('hdmf-experimental')  # remove validation of hdmf-experimental for now
   namespaces = list(sorted(s))

   if len(namespaces) > 0:
      tm = TypeMap(catalog)
      manager = BuildManager(tm)
   else:
      manager = None

   return namespaces, manager, ns_deps


if __name__ == '__main__':
   path = "001_140709EXP_A1.nwb"
   validate_namespaces, manager, chached_namespaces = get_chached_namespaces_to_validate(path)
   with NWBHDF5IO(path, "r", manager=manager) as reader:
      errors = []
      for ns in validate_namespaces:
         errors += validate(io=reader, namespace=ns)
      print("%i errors found" % len(errors))


@tmchartrand
Copy link

I'm getting related errors also, would be great to get this fix integrated!

yarikoptic added a commit that referenced this issue Jun 29, 2022
To overcome problems like presented in dandi/helpdesk#43
this introduces solution proposed by @orugbel in #917 (comment)

Unfortunately there were no release of pynwb with that function yet, so we
are doomed to duplicate code and do it "manually" here for now

Closes #917
@yarikoptic
Copy link
Member

Since NeurodataWithoutBorders/pynwb#1432 is not yet merged&released yet, I guess we are doomed to introduce this into dandi-cli. Submitted #1036 for our consideration ;)

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