catalystneuro/neuroconv

[Bug]: "spikeinterface._get_null_value_for_property" cannot recognize type "int"

Opened this issue · 3 comments

borrepp commented

What happened?

Hello everyone

I'm using neuroconv to export a recording object to an existing NWB. This NWB has extra columns in the electrode table, one of the columns corresponds to integers. When adding the recording, I got the error when the function ran "add_electrodes_to_nwbfile". The error says that there is no default value for integers.

I made it work by adding: "int: int(0)" into the "type_to_default_value" dictionary in the function "_get_null_value_for_property"

type_to_default_value = {list: [], np.ndarray: np.array(np.nan), str: "", float: np.nan, complex: np.nan, int: int(0)}

I think could be useful to add "int" type, especially for cases when reimporting preprocessed recording objects to pre-existing NWB files.

Another option is to allow the variable "null_values_for_properties" as an input to the all the functions that call "add_electrodes_to_nwbfile"

Please let me know whether this temporary fix makes sense.

Best
Pepe

Steps to Reproduce

1) I added new columns to the electrode table, one consisted of an integer number per channel. 
2) I export the NWB file into a new file
3) I read an acquisition group into a spikeinterface recording object
4) I did some filtering & resampling
5) When I tried to import the preprocessed recording object ("spikeinterface.add_recording_to_nwbfile" as LFP) to the NWB from which I extracted it, it did not recognize the "int" type from preexisting columns.

Traceback

No response

Operating System

Windows

Python Executable

Python

Python Version

3.8

Package Versions

No response

Code of Conduct

h-mayorquin commented

Hi, thanks for filling the issue.
Question: is the case that your table should be expanded, that is,you already had some electrodes and now you are adding more? I am asking this because I am wondering if it is the same case you described on #1135

Now, your proposal:

  1. int(0) gives me 0 on my system. I am unsure about this. That functionality was introduced precisely because there is not a natural nan value for the integers. In that case, it falls to the user to determine this. In many cases, the 0 has a clear meaning so it should not be used as default for missing values.

Another option is to allow the variable "null_values_for_properties" as an input to the all the functions that call "add_electrodes_to_nwbfile"

This might be a good idea though. Which function are you calling, add_recording?

borrepp commented

Hello Heberto¡

Thanks for your response. It is not the same case as before. I still have the same electrodes but added extra columns to the electrode table this time. But you are right, 0 (zero) can have a meaning I shouldn't use as a default.

I can change the data type to float, it is only a vector of single digits. It will not impact the size of the file.

About the function, yes, I'm using add_recording, but even if I use add_electrical_series_to_nwbfile directly, that one also calls "add_electrodes_to_nwbfile".

Thanks again for your help.

Best
Pepe

h-mayorquin commented

Yes, that's an option, or to pick an integer number that does not make sense in the specific context of your data (-1) if you are talking about counts or something like that.

  • 👍1