Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
2 tasks done
borrepp opened this issue Nov 27, 2024 · 3 comments
Open
2 tasks done
Labels

Comments

@borrepp
Copy link

borrepp commented Nov 27, 2024

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

@borrepp borrepp added the bug label Nov 27, 2024
@h-mayorquin
Copy link
Collaborator

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
Copy link
Author

borrepp commented Nov 29, 2024

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
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants