hdmf-dev / hdmf

The Hierarchical Data Modeling Framework
http://hdmf.readthedocs.io
Other
46 stars 26 forks source link

get_class to handle datetimes #429

Open bendichter opened 3 years ago

bendichter commented 3 years ago

Handling a datetime attribute requires e.g.

@ObjectMapper.constructor_arg('session_start_time')
    def dateconversion(self, builder, manager):
        datestr = builder.get('session_start_time').data
        date = dateutil_parse(datestr)
        return date

in the io mapper for that class. We would need to implement this for get_class to work on neurodata_types that take datetimes.

bendichter commented 3 years ago

@rly would you help me figure out precisely what test is needed for this? I think we'd have to have a full write-to-disk round trip to make sure that the object mapper is working properly. Is there currently a place to do that for auto-generated API classes?

rly commented 3 weeks ago

This was raised as an issue by @tabedzki.

  1. Attributes with isodatetime/datetime dtype cannot be built:
    Traceback (most recent call last):
      File "/Users/rly/mambaforge/envs/berke2/lib/python3.12/site-packages/hdmf/build/objectmapper.py", line 1020, in __add_attributes
        attr_value, attr_dtype = self.convert_dtype(spec, attr_value)
                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "/Users/rly/mambaforge/envs/berke2/lib/python3.12/site-packages/hdmf/build/objectmapper.py", line 254, in convert_dtype
        ret = spec_dtype_type(value)
              ^^^^^^^^^^^^^^^^^^^^^^
      File "/Users/rly/mambaforge/envs/berke2/lib/python3.12/site-packages/hdmf/build/objectmapper.py", line 89, in _ascii
        raise ValueError("Expected unicode or ascii string, got %s" % type(s))
    ValueError: Expected unicode or ascii string, got <class 'datetime.datetime'>
  2. Datasets with dtype datetime cannot be built
    Traceback (most recent call last):
      File "/Users/rly/mambaforge/envs/berke2/lib/python3.12/site-packages/hdmf/build/objectmapper.py", line 1092, in __add_datasets
        data, dtype = self.convert_dtype(spec, attr_value)
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "/Users/rly/mambaforge/envs/berke2/lib/python3.12/site-packages/hdmf/build/objectmapper.py", line 254, in convert_dtype
        ret = spec_dtype_type(value)
              ^^^^^^^^^^^^^^^^^^^^^^
      File "/Users/rly/mambaforge/envs/berke2/lib/python3.12/site-packages/hdmf/build/objectmapper.py", line 89, in _ascii
        raise ValueError("Expected unicode or ascii string, got %s" % type(s))
    ValueError: Expected unicode or ascii string, got <class 'datetime.datetime'>
  3. Datasets with dtype isodatetime cannot be constructed without the object mapper mentioned by @bendichter above
    Traceback (most recent call last):
      File "/Users/rly/mambaforge/envs/berke2/lib/python3.12/site-packages/hdmf/build/objectmapper.py", line 1345, in construct
        obj = self.__new_container__(cls, builder.source, parent, builder.attributes.get(self.__spec.id_key()),
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "/Users/rly/mambaforge/envs/berke2/lib/python3.12/site-packages/hdmf/build/objectmapper.py", line 1358, in __new_container__
        obj.__init__(**kwargs)
      File "/Users/rly/mambaforge/envs/berke2/lib/python3.12/site-packages/hdmf/utils.py", line 667, in func_call
        pargs = _check_args(args, kwargs)
                ^^^^^^^^^^^^^^^^^^^^^^^^^
      File "/Users/rly/mambaforge/envs/berke2/lib/python3.12/site-packages/hdmf/utils.py", line 660, in _check_args
        raise ExceptionType(msg)
    TypeError: CustomClassGenerator.set_init.<locals>.__init__: incorrect type for 'session_end_time' (got 'str', expected 'datetime or date')

This makes it extremely difficult to use the dtype isodatetime or datetime in extensions.

A workaround is to use dtype text and have code around the extension to convert between datetime and str in python.

I will try to address this next week.

tabedzki commented 3 days ago

@rly where does this sit on your to-do list?