[Bug]: Loading nwbs created with older pynwb version fails due to '/' or ':' in Device.model name

[calderast]

What happened?

I created this nwbfile with an older pynwb version, where the device model name string has special characters ('MFC_200/250-0.66_40mm_MF2.5_FLT'). When the file tries to be loaded with a newer pynwb version, the remapping fails:

[/home/scrater/miniforge3/envs/spyglass/lib/python3.10/site-packages/hdmf/build/objectmapper.py:1401](https://vscode-remote+ssh-002dremote-002bbreeze-002ecin-002eucsf-002eedu.vscode-resource.vscode-cdn.net/home/scrater/miniforge3/envs/spyglass/lib/python3.10/site-packages/hdmf/build/objectmapper.py:1401): UserWarning: Device.model was detected as a string, but NWB 2.9 specifies Device.model as a link to a DeviceModel. Remapping "MFC_200/250-0.66_40mm_MF2.5_FLT" to a new DeviceModel.
  override = self.__get_override_carg(argname, builder, manager)

I patched this by adding this snippet to the init.py of my schema for loading these nwbs:

# PyNWB 3.x introduced DeviceModel, which crashes when reading NWB files whose
# device names contain '/' or ':' (e.g. 'MFC_200/250-0.66_40mm_MF2.5_FLT').
# Patch DeviceMapper to silently skip invalid device model names on read.
try:
    from pynwb.io.device import DeviceMapper

    _orig_model_fn = DeviceMapper.constructor_args.get("model")

    if _orig_model_fn:
        def _safe_model_fn(self, builder, manager):
            try:
                return _orig_model_fn(self, builder, manager)
            except ValueError:
                return None
        DeviceMapper.constructor_args["model"] = _safe_model_fn
except Exception:
    pass  # older pynwb without DeviceMapper — no patch needed

Steps to Reproduce

key = {"nwb_file_name": "IM-1478_20220725_.nwb"}
df = (HexMazeJunctionDecode & key).fetch1_dataframe()

Traceback

---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
Cell In[2], line 1
----> 1 df = (HexMazeJunctionDecode & key).fetch1_dataframe()

File ~/dev/Hex-maze-spyglass/spyglass_hexmaze/hex_maze_decoding.py:1415, in HexMazeJunctionDecode.fetch1_dataframe(self)
   1414 def fetch1_dataframe(self):
-> 1415     return self.fetch_nwb()[0]["junction_decode"].set_index("time")

File ~/dev/spyglass/src/spyglass/utils/mixins/fetch.py:336, in FetchMixin.fetch_nwb(self, *attrs, **kwargs)
    333 rec_dicts = self._execute_nwb_query(table, tbl_attr, *attrs, **kwargs)
    335 # Process object_id fields if present
--> 336 return self._process_object_ids(rec_dicts, *attrs)

File ~/dev/spyglass/src/spyglass/utils/mixins/fetch.py:261, in FetchMixin._process_object_ids(self, rec_dicts, *attrs)
    259 ret = []
    260 for rec_dict in rec_dicts:
--> 261     nwbf = get_nwb_file(rec_dict.pop("nwb2load_filepath"))
    262     # for each attr that contains substring 'object_id', store key-value:
    263     # attr name to NWB object
    264     # remove '_object_id' from attr name
    265     nwb_objs = {
    266         id_attr.replace("_object_id", ""): self._get_nwb_object(
    267             nwbf.objects, rec_dict[id_attr]
   (...)
    270         if "object_id" in id_attr and rec_dict[id_attr] != ""
    271     }

File ~/dev/spyglass/src/spyglass/utils/nwb_helper_fn.py:88, in get_nwb_file(nwb_file_path, query_expression)
     85     return nwbfile
     87 if os.path.exists(nwb_file_path):
---> 88     return _open_nwb_file(nwb_file_path)
     90 logger.info(
     91     f"NWB file not found locally; checking kachery for {nwb_file_path}"
     92 )
     94 from ..sharing.sharing_kachery import AnalysisNwbfileKachery

File ~/dev/spyglass/src/spyglass/utils/nwb_helper_fn.py:29, in _open_nwb_file(nwb_file_path, source)
     27 if source == "local":
     28     io = pynwb.NWBHDF5IO(path=nwb_file_path, mode="r", load_namespaces=True)
---> 29     nwbfile = io.read()
     30 elif source == "dandi":
     31     from ..common.common_dandi import DandiPath

File ~/miniforge3/envs/spyglass/lib/python3.10/site-packages/hdmf/utils.py:592, in docval.<locals>.dec.<locals>.func_call(*args, **kwargs)
    590 def func_call(*args, **kwargs):
    591     pargs = _check_args(args, kwargs)
--> 592     return func(args[0], **pargs)

File ~/miniforge3/envs/spyglass/lib/python3.10/site-packages/pynwb/__init__.py:489, in NWBHDF5IO.read(self, **kwargs)
    486         raise TypeError("NWB version %s not supported. PyNWB supports NWB files version 2 and above." %
    487                         str(file_version_str))
    488 # read the file
--> 489 file = super().read(**kwargs)
    490 return file

File ~/miniforge3/envs/spyglass/lib/python3.10/site-packages/hdmf/backends/hdf5/h5tools.py:458, in HDF5IO.read(self, **kwargs)
    455     raise UnsupportedOperation("Cannot read from file %s in mode '%s'. Please use mode 'r', 'r+', or 'a'."
    456                                % (self.source, self.__mode))
    457 try:
--> 458     return super().read(**kwargs)
    459 except UnsupportedOperation as e:
    460     if str(e) == 'Cannot build data. There are no values.':  # pragma: no cover

File ~/miniforge3/envs/spyglass/lib/python3.10/site-packages/hdmf/utils.py:592, in docval.<locals>.dec.<locals>.func_call(*args, **kwargs)
    590 def func_call(*args, **kwargs):
    591     pargs = _check_args(args, kwargs)
--> 592     return func(args[0], **pargs)

File ~/miniforge3/envs/spyglass/lib/python3.10/site-packages/hdmf/backends/io.py:60, in HDMFIO.read(self, **kwargs)
     57 if all(len(v) == 0 for v in f_builder.values()):
     58     # TODO also check that the keys are appropriate. print a better error message
     59     raise UnsupportedOperation('Cannot build data. There are no values.')
---> 60 container = self.__manager.construct(f_builder)
     61 container.read_io = self
     62 if self.herd_path is not None:

File ~/miniforge3/envs/spyglass/lib/python3.10/site-packages/hdmf/utils.py:592, in docval.<locals>.dec.<locals>.func_call(*args, **kwargs)
    590 def func_call(*args, **kwargs):
    591     pargs = _check_args(args, kwargs)
--> 592     return func(args[0], **pargs)

File ~/miniforge3/envs/spyglass/lib/python3.10/site-packages/hdmf/build/manager.py:286, in BuildManager.construct(self, **kwargs)
    282     result = self.__type_map.construct(builder, self, parent)
    283 else:
    284     # we are at the top of the hierarchy,
    285     # so it must be time to resolve parents
--> 286     result = self.__type_map.construct(builder, self, None)
    287     self.__resolve_parents(result)
    288 self.prebuilt(result, builder)

File ~/miniforge3/envs/spyglass/lib/python3.10/site-packages/hdmf/utils.py:592, in docval.<locals>.dec.<locals>.func_call(*args, **kwargs)
    590 def func_call(*args, **kwargs):
    591     pargs = _check_args(args, kwargs)
--> 592     return func(args[0], **pargs)

File ~/miniforge3/envs/spyglass/lib/python3.10/site-packages/hdmf/build/manager.py:814, in TypeMap.construct(self, **kwargs)
    812     raise ValueError('No ObjectMapper found for builder of type %s' % dt)
    813 else:
--> 814     return obj_mapper.construct(builder, build_manager, parent)

File ~/miniforge3/envs/spyglass/lib/python3.10/site-packages/hdmf/utils.py:592, in docval.<locals>.dec.<locals>.func_call(*args, **kwargs)
    590 def func_call(*args, **kwargs):
    591     pargs = _check_args(args, kwargs)
--> 592     return func(args[0], **pargs)

File ~/miniforge3/envs/spyglass/lib/python3.10/site-packages/pynwb/io/file.py:161, in NWBFileMap.construct(self, **kwargs)
    157         builder.groups['bands'].attributes['namespace'] = 'core'
    159 apply_to_child_builders(nwbfile_builder, [update_builder_frequency_bands_table])
--> 161 return super().construct(**kwargs)

File ~/miniforge3/envs/spyglass/lib/python3.10/site-packages/hdmf/utils.py:592, in docval.<locals>.dec.<locals>.func_call(*args, **kwargs)
    590 def func_call(*args, **kwargs):
    591     pargs = _check_args(args, kwargs)
--> 592     return func(args[0], **pargs)

File ~/miniforge3/envs/spyglass/lib/python3.10/site-packages/hdmf/build/objectmapper.py:1380, in ObjectMapper.construct(self, **kwargs)
   1378 cls = manager.get_cls(builder)
   1379 # gather all subspecs
-> 1380 subspecs = self.__get_subspec_values(builder, self.spec, manager)
   1381 # get the constructor argument that each specification corresponds to
   1382 const_args = dict()

File ~/miniforge3/envs/spyglass/lib/python3.10/site-packages/hdmf/build/objectmapper.py:1309, in ObjectMapper.__get_subspec_values(self, builder, spec, manager)
   1307                 ret[subspec] = self.__flatten(sub_builder, subspec, manager)
   1308     # now process groups and datasets
-> 1309     self.__get_sub_builders(groups, spec.groups, manager, ret)
   1310     self.__get_sub_builders(datasets, spec.datasets, manager, ret)
   1311 elif isinstance(spec, DatasetSpec):

File ~/miniforge3/envs/spyglass/lib/python3.10/site-packages/hdmf/build/objectmapper.py:1360, in ObjectMapper.__get_sub_builders(self, sub_builders, subspecs, manager, ret)
   1357     continue
   1358 if dt is None:
   1359     # recurse
-> 1360     ret.update(self.__get_subspec_values(sub_builder, subspec, manager))
   1361 else:
   1362     ret[subspec] = manager.construct(sub_builder)

File ~/miniforge3/envs/spyglass/lib/python3.10/site-packages/hdmf/build/objectmapper.py:1309, in ObjectMapper.__get_subspec_values(self, builder, spec, manager)
   1307                 ret[subspec] = self.__flatten(sub_builder, subspec, manager)
   1308     # now process groups and datasets
-> 1309     self.__get_sub_builders(groups, spec.groups, manager, ret)
   1310     self.__get_sub_builders(datasets, spec.datasets, manager, ret)
   1311 elif isinstance(spec, DatasetSpec):

File ~/miniforge3/envs/spyglass/lib/python3.10/site-packages/hdmf/build/objectmapper.py:1360, in ObjectMapper.__get_sub_builders(self, sub_builders, subspecs, manager, ret)
   1357     continue
   1358 if dt is None:
   1359     # recurse
-> 1360     ret.update(self.__get_subspec_values(sub_builder, subspec, manager))
   1361 else:
   1362     ret[subspec] = manager.construct(sub_builder)

File ~/miniforge3/envs/spyglass/lib/python3.10/site-packages/hdmf/build/objectmapper.py:1309, in ObjectMapper.__get_subspec_values(self, builder, spec, manager)
   1307                 ret[subspec] = self.__flatten(sub_builder, subspec, manager)
   1308     # now process groups and datasets
-> 1309     self.__get_sub_builders(groups, spec.groups, manager, ret)
   1310     self.__get_sub_builders(datasets, spec.datasets, manager, ret)
   1311 elif isinstance(spec, DatasetSpec):

File ~/miniforge3/envs/spyglass/lib/python3.10/site-packages/hdmf/build/objectmapper.py:1352, in ObjectMapper.__get_sub_builders(self, sub_builders, subspecs, manager, ret)
   1350     sub_builder = builder_dt.get(dt)
   1351     if sub_builder is not None:
-> 1352         sub_builder = self.__flatten(sub_builder, subspec, manager)
   1353         ret[subspec] = sub_builder
   1354 else:

File ~/miniforge3/envs/spyglass/lib/python3.10/site-packages/hdmf/build/objectmapper.py:1365, in ObjectMapper.__flatten(self, sub_builder, subspec, manager)
   1364 def __flatten(self, sub_builder, subspec, manager):
-> 1365     tmp = [manager.construct(b) for b in sub_builder]
   1366     if len(tmp) == 1 and not subspec.is_many():
   1367         tmp = tmp[0]

File ~/miniforge3/envs/spyglass/lib/python3.10/site-packages/hdmf/build/objectmapper.py:1365, in <listcomp>(.0)
   1364 def __flatten(self, sub_builder, subspec, manager):
-> 1365     tmp = [manager.construct(b) for b in sub_builder]
   1366     if len(tmp) == 1 and not subspec.is_many():
   1367         tmp = tmp[0]

File ~/miniforge3/envs/spyglass/lib/python3.10/site-packages/hdmf/utils.py:592, in docval.<locals>.dec.<locals>.func_call(*args, **kwargs)
    590 def func_call(*args, **kwargs):
    591     pargs = _check_args(args, kwargs)
--> 592     return func(args[0], **pargs)

File ~/miniforge3/envs/spyglass/lib/python3.10/site-packages/hdmf/build/manager.py:282, in BuildManager.construct(self, **kwargs)
    280 if parent_builder is not None:
    281     parent = self._get_proxy_builder(parent_builder)
--> 282     result = self.__type_map.construct(builder, self, parent)
    283 else:
    284     # we are at the top of the hierarchy,
    285     # so it must be time to resolve parents
    286     result = self.__type_map.construct(builder, self, None)

File ~/miniforge3/envs/spyglass/lib/python3.10/site-packages/hdmf/utils.py:592, in docval.<locals>.dec.<locals>.func_call(*args, **kwargs)
    590 def func_call(*args, **kwargs):
    591     pargs = _check_args(args, kwargs)
--> 592     return func(args[0], **pargs)

File ~/miniforge3/envs/spyglass/lib/python3.10/site-packages/hdmf/build/manager.py:814, in TypeMap.construct(self, **kwargs)
    812     raise ValueError('No ObjectMapper found for builder of type %s' % dt)
    813 else:
--> 814     return obj_mapper.construct(builder, build_manager, parent)

File ~/miniforge3/envs/spyglass/lib/python3.10/site-packages/hdmf/utils.py:592, in docval.<locals>.dec.<locals>.func_call(*args, **kwargs)
    590 def func_call(*args, **kwargs):
    591     pargs = _check_args(args, kwargs)
--> 592     return func(args[0], **pargs)

File ~/miniforge3/envs/spyglass/lib/python3.10/site-packages/hdmf/build/objectmapper.py:1401, in ObjectMapper.construct(self, **kwargs)
   1399 for const_arg in get_docval(cls.__init__):
   1400     argname = const_arg['name']
-> 1401     override = self.__get_override_carg(argname, builder, manager)
   1402     if override is not None:
   1403         val = override

File ~/miniforge3/envs/spyglass/lib/python3.10/site-packages/hdmf/build/objectmapper.py:590, in ObjectMapper.__get_override_carg(self, *args)
    588     self.logger.debug("        Calling override function for constructor argument '%s'" % name)
    589     func = self.constructor_args[name]
--> 590     return func(self, *remaining_args)
    591 return None

File ~/miniforge3/envs/spyglass/lib/python3.10/site-packages/pynwb/io/device.py:41, in DeviceMapper.model_carg(self, builder, manager)
     36     # replace the model string with a DeviceModel object using the model name and device attributes 
     37     device_model_attributes = dict(name=model_builder,
     38                                    description=builder.attributes.get('description'),
     39                                    manufacturer=builder.attributes.get('manufacturer', ''),
     40                                    model_number=builder.attributes.get('model_number'))
---> 41     model = DeviceModel(**device_model_attributes)
     43     return model
     45 return None

File ~/miniforge3/envs/spyglass/lib/python3.10/site-packages/hdmf/utils.py:592, in docval.<locals>.dec.<locals>.func_call(*args, **kwargs)
    590 def func_call(*args, **kwargs):
    591     pargs = _check_args(args, kwargs)
--> 592     return func(args[0], **pargs)

File ~/miniforge3/envs/spyglass/lib/python3.10/site-packages/pynwb/device.py:112, in DeviceModel.__init__(self, **kwargs)
     97 @docval(
     98     {'name': 'name', 'type': str, 'doc': 'The name of this device model'},
     99     {'name': 'manufacturer', 'type': str,
   (...)
    109 )
    110 def __init__(self, **kwargs):
    111     manufacturer, model_number, description = popargs('manufacturer', 'model_number', 'description', kwargs)
--> 112     super().__init__(**kwargs)
    113     self.manufacturer = manufacturer
    114     self.model_number = model_number

File ~/miniforge3/envs/spyglass/lib/python3.10/site-packages/hdmf/utils.py:592, in docval.<locals>.dec.<locals>.func_call(*args, **kwargs)
    590 def func_call(*args, **kwargs):
    591     pargs = _check_args(args, kwargs)
--> 592     return func(args[0], **pargs)

File ~/miniforge3/envs/spyglass/lib/python3.10/site-packages/hdmf/container.py:327, in AbstractContainer.__init__(self, **kwargs)
    325 name = getargs('name', kwargs)
    326 if ('/' in name or ':' in name) and not self._in_construct_mode:
--> 327     raise ValueError(f"name '{name}' cannot contain a '/' or ':'")
    328 self.__name = name
    329 self.__field_values = dict()

ValueError: name 'MFC_200/250-0.66_40mm_MF2.5_FLT' cannot contain a '/' or ':'

Operating System

Linux

Python Executable

Conda

Python Version

3.10

Package Versions

environment_for_issue.txt

Code of Conduct

• I agree to follow this project's Code of Conduct
• Have you checked the Contributing document?
• Have you ensured this bug was not already reported?

[rly]

Thanks for making the issue ticket @calderast (following up from Slack).

I'm surprised that this is triggered on read because the ValueError is guarded by self._in_construct_mode which should be True. I'll investigate. Either way, setting the object name this way will create problems when writing the remapped NWB file in memory to disk or exporting it to a new file.

Would it be acceptable to you if the remapping set the name of the new DeviceModel object to a modified version of the original Device.model attribute, where all "/" and ":" are replaced with "_"? e.g. "MFC_200/250-0.66_40mm_MF2.5_FLT" -> "MFC_200_250-0.66_40mm_MF2.5_FLT"

[calderast]

Thanks for making the issue ticket @calderast (following up from Slack).

I'm surprised that this is triggered on read because the ValueError is guarded by self._in_construct_mode which should be True. I'll investigate. Either way, setting the object name this way will create problems when writing the remapped NWB file in memory to disk or exporting it to a new file.

Would it be acceptable to you if the remapping set the name of the new DeviceModel object to a modified version of the original Device.model attribute, where all "/" and ":" are replaced with "_"? e.g. "MFC_200/250-0.66_40mm_MF2.5_FLT" -> "MFC_200_250-0.66_40mm_MF2.5_FLT"

yeah, an underscore replace would be fine!