ChangLabUcsf / ndx-electrical-stim

An NWB extension for storing electrical stimulation waveforms and metadata for neural stimulation.
Other
2 stars 1 forks source link

Issue adding metadata field - 'bytes' object has no attribute 'name' #4

Closed jessierliu closed 4 years ago

jessierliu commented 4 years ago

@bendichter I'm having an issue actually adding the metadata attribute to the StimSeries object. I've defined it's type as bytes, so I change the JSON serialized string to utf-8 encoding prior to actually filling out that field.

If I leave this line below commented out, then there are no issues obviously because it does not actually get added to the file. https://github.com/ChangLabUcsf/ndx-electrical-stim/blob/e0b1b1ee5c0cb20cbcff5f742d9a88da59a14242/src/pynwb/ndx_electrical_stim/ndx_electrical_stim.py#L93

Once un-commented, it creates the following error when trying to create the StimSeries:

AttributeError                            Traceback (most recent call last)
<ipython-input-5-53d7a7458d9e> in <module>
      5     unit='amp',
      6     rate=200.,
----> 7     metadata=stim_meta
      8 )
      9 

/anaconda3/envs/ecog_vis_dev/lib/python3.7/site-packages/hdmf/utils.py in func_call(*args, **kwargs)
    436                         raise_from(ExceptionType(msg), None)
    437 
--> 438                 return func(self, **parsed['args'])
    439         else:
    440             def func_call(*args, **kwargs):

~/ChangLab/projects/ndx-electrical-stim/src/pynwb/ndx_electrical_stim/ndx_electrical_stim.py in __init__(self, **kwargs)
     91         super(StimSeries, self).__init__(name, data, unit, **kwargs)
     92         self.electrodes = electrodes
---> 93         self.metadata = metadata
     94 

/anaconda3/envs/ecog_vis_dev/lib/python3.7/site-packages/hdmf/container.py in container_setter(self, val)
    264 
    265                 def container_setter(self, val):
--> 266                     ret[idx2](self, val)
    267                     if val is not None:
    268                         if isinstance(val, (tuple, list)):

/anaconda3/envs/ecog_vis_dev/lib/python3.7/site-packages/hdmf/container.py in container_setter(self, val)
    254 
    255                 def container_setter(self, val):
--> 256                     if val is not None and val.name != name:
    257                         msg = "%s field on %s must be named '%s'" % (field['name'], self.__class__.__name__, name)
    258                         raise ValueError(msg)

AttributeError: 'bytes' object has no attribute 'name'

This is the code I'm using to create the StimSeries:

import json
import os
from datetime import datetime

import numpy as np
from pynwb import NWBHDF5IO, NWBFile
from ndx_electrical_stim import StimSeries

nwbfile = NWBFile('description', 'id', datetime.now().astimezone())
device = nwbfile.create_device('device_test')
group = nwbfile.create_electrode_group(
        name='electrodes',
        description='desc',
        device=device,
        location='loc')

for _ in range(3):
    nwbfile.add_electrode(x=np.nan, y=np.nan, z=np.nan, imp=np.nan,
                        location='', filtering='', group=group)

waveform = np.random.randn(500, 2)

elec_region = nwbfile.create_electrode_table_region(
    [0, 2],
    'electrodes'
)

stim_params = {
    "run1": {
        "frequency": 10,
        "amplitude": 5,
        "pulse_width": 2
    },
    "run2": {
        "frequency": 10,
        "amplitude": 5,
        "pulse_width": 2,
        "other_param": "value"
    }
}

stim_meta = json.dumps(stim_params).encode(encoding='utf-8')

ss = StimSeries(
    'stim',
    data=waveform,
    electrodes=elec_region,
    unit='amp',
    rate=200.,
    metadata=stim_meta
)

I figure there's got to be something obvious that I'm missing when defining the datatypes?

bendichter commented 4 years ago

Define it as 'text' not bytes and input a str

On Tue, Nov 19, 2019, 7:24 PM Jessie Liu notifications@github.com wrote:

@bendichter https://github.com/bendichter I'm having an issue actually adding the metadata attribute to the StimSeries object. I've defined it's type as bytes, so I change the JSON serialized string to utf-8 encoding prior to actually filling out that field.

If I leave this line below commented out, then there are no issues obviously because it does not actually get added to the file.

https://github.com/ChangLabUcsf/ndx-electrical-stim/blob/e0b1b1ee5c0cb20cbcff5f742d9a88da59a14242/src/pynwb/ndx_electrical_stim/ndx_electrical_stim.py#L93

Once un-commented, it creates the following error when trying to create the StimSeries:

AttributeError Traceback (most recent call last)

in 5 unit='amp', 6 rate=200., ----> 7 metadata=stim_meta 8 ) 9 /anaconda3/envs/ecog_vis_dev/lib/python3.7/site-packages/hdmf/utils.py in func_call(*args, **kwargs) 436 raise_from(ExceptionType(msg), None) 437 --> 438 return func(self, **parsed['args']) 439 else: 440 def func_call(*args, **kwargs): ~/ChangLab/projects/ndx-electrical-stim/src/pynwb/ndx_electrical_stim/ndx_electrical_stim.py in __init__(self, **kwargs) 91 super(StimSeries, self).__init__(name, data, unit, **kwargs) 92 self.electrodes = electrodes ---> 93 self.metadata = metadata 94 /anaconda3/envs/ecog_vis_dev/lib/python3.7/site-packages/hdmf/container.py in container_setter(self, val) 264 265 def container_setter(self, val): --> 266 ret[idx2](self, val) 267 if val is not None: 268 if isinstance(val, (tuple, list)): /anaconda3/envs/ecog_vis_dev/lib/python3.7/site-packages/hdmf/container.py in container_setter(self, val) 254 255 def container_setter(self, val): --> 256 if val is not None and val.name != name: 257 msg = "%s field on %s must be named '%s'" % (field['name'], self.__class__.__name__, name) 258 raise ValueError(msg) AttributeError: 'bytes' object has no attribute 'name' This is the code I'm using to create the StimSeries: import json import os from datetime import datetime import numpy as np from pynwb import NWBHDF5IO, NWBFile from ndx_electrical_stim import StimSeries nwbfile = NWBFile('description', 'id', datetime.now().astimezone()) device = nwbfile.create_device('device_test') group = nwbfile.create_electrode_group( name='electrodes', description='desc', device=device, location='loc') for _ in range(3): nwbfile.add_electrode(x=np.nan, y=np.nan, z=np.nan, imp=np.nan, location='', filtering='', group=group) waveform = np.random.randn(500, 2) elec_region = nwbfile.create_electrode_table_region( [0, 2], 'electrodes' ) stim_params = { "run1": { "frequency": 10, "amplitude": 5, "pulse_width": 2 }, "run2": { "frequency": 10, "amplitude": 5, "pulse_width": 2, "other_param": "value" } } stim_meta = json.dumps(stim_params).encode(encoding='utf-8') ss = StimSeries( 'stim', data=waveform, electrodes=elec_region, unit='amp', rate=200., metadata=stim_meta ) I figure there's got to be something obvious that I'm missing when defining the datatypes? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub , or unsubscribe .
jessierliu commented 4 years ago

I had tried this as well yesterday. My question then became how do I define the type of metadata in the docval? That is:

https://github.com/ChangLabUcsf/ndx-electrical-stim/blob/e0b1b1ee5c0cb20cbcff5f742d9a88da59a14242/src/pynwb/ndx_electrical_stim/ndx_electrical_stim.py#L53-L59

I haven't encountered "text" as a datatype before. If I define the type as 'text' (which I don't think I should be doing), then at the same point as the above code, I get a TypeError that it expected 'text' and got 'str'.

If I define the type in the docval as str, I get a different error:

---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-5-53d7a7458d9e> in <module>
      5     unit='amp',
      6     rate=200.,
----> 7     metadata=stim_meta
      8 )
      9 

/anaconda3/envs/ecog_vis_dev/lib/python3.7/site-packages/hdmf/utils.py in func_call(*args, **kwargs)
    436                         raise_from(ExceptionType(msg), None)
    437 
--> 438                 return func(self, **parsed['args'])
    439         else:
    440             def func_call(*args, **kwargs):

~/ChangLab/projects/ndx-electrical-stim/src/pynwb/ndx_electrical_stim/ndx_electrical_stim.py in __init__(self, **kwargs)
     91         super(StimSeries, self).__init__(name, data, unit, **kwargs)
     92         self.electrodes = electrodes
---> 93         self.metadata = metadata
     94 

/anaconda3/envs/ecog_vis_dev/lib/python3.7/site-packages/hdmf/container.py in container_setter(self, val)
    264 
    265                 def container_setter(self, val):
--> 266                     ret[idx2](self, val)
    267                     if val is not None:
    268                         if isinstance(val, (tuple, list)):

/anaconda3/envs/ecog_vis_dev/lib/python3.7/site-packages/hdmf/container.py in container_setter(self, val)
    254 
    255                 def container_setter(self, val):
--> 256                     if val is not None and val.name != name:
    257                         msg = "%s field on %s must be named '%s'" % (field['name'], self.__class__.__name__, name)
    258                         raise ValueError(msg)

AttributeError: 'str' object has no attribute 'name'

I had thought it might be an issue of defining metadata as an attribute vs a dataset, but I get the AttributeError regardless.

bendichter commented 4 years ago

You should be able to get away with using get_class here. See https://github.com/ben-dichter-consulting/ndx-bipolar-scheme/blob/4da9662614b92db43ae1066540423f1469418ac7/src/pynwb/ndx_bipolar_referencing/__init__.py#L24

jessierliu commented 4 years ago

Ah, sweet! That worked, thanks.

jessierliu commented 4 years ago

It seems though that by using get_class, the variable checks, such as the one below, do not get executed.

https://github.com/ChangLabUcsf/ndx-electrical-stim/blob/1816811ca7ab71fb6662a3df9c5c068b60c9bdcd/src/pynwb/ndx_electrical_stim/ndx_electrical_stim.py#L72-L75

Confused because it does seem that electrodes and metadata get added to StimSeries which happens just a few lines below it.

https://github.com/ChangLabUcsf/ndx-electrical-stim/blob/1816811ca7ab71fb6662a3df9c5c068b60c9bdcd/src/pynwb/ndx_electrical_stim/ndx_electrical_stim.py#L103-L105

bendichter commented 4 years ago

get_class completely replaces that manually created class, so you are right that no checks are done. You might be able to get away with something like:


my_class = get_class(...)

def my_init(self, **kwargs):
    if kwargs['unit'] not in ['amp', 'volt']:
            raise ValueError('Stimulation waveform unit must be "amp" or '
                             '"volt".')
    my_class.__init__(self, **kwargs)

my_class.__init__ = my_init
``
bendichter commented 4 years ago

you can also see the correct docval usage by inspecting my_class.docval on the generated class (or something like that)

jessierliu commented 4 years ago

Ah I see. Instead of directly using that class generated by get_class, I used it as a base class to build on the __init__ that I wanted into the actual StimSeries that gets returned. I haven't experienced any issues with it, but it does seem slightly hack-y.

bendichter commented 4 years ago

That should work!

On Thu, Nov 21, 2019, 5:04 PM Jessie Liu notifications@github.com wrote:

Ah I see. Instead of directly using that class generated by get_class, I used it as a base class to build on the init that I wanted into the actual StimSeries that gets returned. I haven't experienced any issues with it, but it does seem slightly hack-y.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ChangLabUcsf/ndx-electrical-stim/issues/4?email_source=notifications&email_token=AAGOEEXKZSU7JDKMZO3RRSTQU4VYXA5CNFSM4JPMWRH2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEE4FWHI#issuecomment-557341469, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGOEERN2GYDMXURK7OBCCTQU4VYXANCNFSM4JPMWRHQ .