NeurodataWithoutBorders / matnwb

A Matlab interface for reading and writing NWB files
BSD 2-Clause "Simplified" License
49 stars 32 forks source link

failing validation of matnwb-generated files when test schemas in namespace #353

Closed cechava closed 2 years ago

cechava commented 2 years ago

This issue is independent of other validation issues and probably only will affect a small number of matnwb users. Validation of matnwb-generated files fails when the following extensions are in the namespace: anon, rrs, and cs. These extensions are found under +tests/+unit folder and are generated when running unit tests. Exclusion of these from the cached namespace resolves the problem. The full traceback of a file generated when these are in the namespace is below.

$ python -m pynwb.validate ophys_tutorial.nwb 
Traceback (most recent call last):
  File "/Users/cesar/anaconda3/envs/pynwb-dev/lib/python3.9/runpy.py", line 197, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/Users/cesar/anaconda3/envs/pynwb-dev/lib/python3.9/runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "/Users/cesar/anaconda3/envs/pynwb-dev/lib/python3.9/site-packages/pynwb/validate.py", line 136, in <module>
    main()
  File "/Users/cesar/anaconda3/envs/pynwb-dev/lib/python3.9/site-packages/pynwb/validate.py", line 72, in main
    ns_deps = NWBHDF5IO.load_namespaces(catalog, path)
  File "/Users/cesar/anaconda3/envs/pynwb-dev/lib/python3.9/site-packages/hdmf/utils.py", line 583, in func_call
    return func(args[0], **pargs)
  File "/Users/cesar/anaconda3/envs/pynwb-dev/lib/python3.9/site-packages/hdmf/backends/hdf5/h5tools.py", line 147, in load_namespaces
    return cls.__load_namespaces(namespace_catalog, namespaces, open_file_obj)
  File "/Users/cesar/anaconda3/envs/pynwb-dev/lib/python3.9/site-packages/hdmf/backends/hdf5/h5tools.py", line 181, in __load_namespaces
    d.update(namespace_catalog.load_namespaces(cls.__ns_spec_path, reader=reader))
  File "/Users/cesar/anaconda3/envs/pynwb-dev/lib/python3.9/site-packages/hdmf/utils.py", line 583, in func_call
    return func(args[0], **pargs)
  File "/Users/cesar/anaconda3/envs/pynwb-dev/lib/python3.9/site-packages/hdmf/spec/namespace.py", line 538, in load_namespaces
    ret[ns['name']] = self.__load_namespace(ns, reader, resolve=resolve)
  File "/Users/cesar/anaconda3/envs/pynwb-dev/lib/python3.9/site-packages/hdmf/spec/namespace.py", line 448, in __load_namespace
    self.__load_spec_file(reader, s['source'], catalog, types_to_load=types_to_load, resolve=resolve)
  File "/Users/cesar/anaconda3/envs/pynwb-dev/lib/python3.9/site-packages/hdmf/spec/namespace.py", line 402, in __load_spec_file
    temp_dict = {k: None for k in __reg_spec(self.__group_spec_cls, spec_dict)}
  File "/Users/cesar/anaconda3/envs/pynwb-dev/lib/python3.9/site-packages/hdmf/spec/namespace.py", line 388, in __reg_spec
    spec_obj = spec_cls.build_spec(spec_dict)
  File "/Users/cesar/anaconda3/envs/pynwb-dev/lib/python3.9/site-packages/hdmf/spec/spec.py", line 93, in build_spec
    vargs = cls.build_const_args(spec_dict)
  File "/Users/cesar/anaconda3/envs/pynwb-dev/lib/python3.9/site-packages/pynwb/spec.py", line 105, in build_const_args
    ret = proxy.build_const_args(spec_dict)
  File "/Users/cesar/anaconda3/envs/pynwb-dev/lib/python3.9/site-packages/hdmf/spec/spec.py", line 1269, in build_const_args
    ret['datasets'] = list(map(cls.dataset_spec_cls().build_spec, ret['datasets']))
  File "/Users/cesar/anaconda3/envs/pynwb-dev/lib/python3.9/site-packages/hdmf/spec/spec.py", line 99, in build_spec
    return cls(**kwargs)
  File "/Users/cesar/anaconda3/envs/pynwb-dev/lib/python3.9/site-packages/hdmf/utils.py", line 582, in func_call
    pargs = _check_args(args, kwargs)
  File "/Users/cesar/anaconda3/envs/pynwb-dev/lib/python3.9/site-packages/hdmf/utils.py", line 575, in _check_args
    raise ExceptionType(msg)
TypeError: NWBDatasetSpec.__init__: missing argument 'doc'
cechava commented 2 years ago

@ln-vidrio let me know your thoughts on this. Maybe just removing the namespaces at the end of the unit tests would be easiest.

lawrence-mbf commented 2 years ago

@cechava I agree, we'll have to do a full cleanup of the cache and generated classes in the unit tests.

cechava commented 2 years ago

Given that some NWB files already have these namespaces cached (e.g., this DANDIset). It would also make sense to modify NwbRead to ignore these test namespaces.

lawrence-mbf commented 2 years ago

Honestly since we're going to definitely have a step to clean up these classes, it wouldn't hurt to also delete them from cache. I'd rather not include more special "names" if I don't have to and it may occur that a user will need to use one of our testing namespace names (not just what we have now, but also unit tests in the future).