datajoint-company / dj-nwb-li-daie-2015-2016 Goto Github PK

In the below code, if there is no stop time associated with a trial, the value 5.0 is entered instead. Why 5.0? Using 5.0 would confuse any algorithms that uses this column to segment and visualize the data. np.nan seems more appropriate. If there is a reason behind using 5.0, please add it to the description of the stop_time column or the trials table.

https://github.com/vathes/DJ-NWB-Li-Daie-2015-2016/blob/ae69335a23e6a3c0c3809a54de5480baf8848712/pipeline/export/datajoint_to_nwb.py#L221

The electrode device is currently just an obscure part number, e.g. "A4x8-5mm-100-200-177". It would be helpful to have a description of what the device actually is. For Li et al., 2015, you could set the device description to "NeuroNexus silicon probes (part no. A4x8-5mm-100-200-177). The 32 channel voltage signals were multiplexed, recorded on a PCI6133 board at 312.5 kHz (National instrument), and digitized at 14 bit. The signals were demultiplexed into the 32 voltage traces, sampled at 19531.25 Hz and stored for offline analyses."

https://github.com/vathes/DJ-NWB-Li-Daie-2015-2016/blob/ae69335a23e6a3c0c3809a54de5480baf8848712/pipeline/export/datajoint_to_nwb.py#L96

In the below code, a lick trace is added as a TimeSeries to the BehavioralTimeSeries data interface. I see that the units are 'a.u.' but what do the data values represent? They are floating point values between 0.11 and 0.15.

Please add a description argument for create_time_series that describes what the values represent so that they can be interpreted by other users.

https://github.com/vathes/DJ-NWB-Li-Daie-2015-2016/blob/ae69335a23e6a3c0c3809a54de5480baf8848712/pipeline/export/datajoint_to_nwb.py#L141-L148

File ANM278733_2015-05-31_3.nwb cannot be read. It is also only 181 KB instead of around 200 MB like the other files. These suggest that something went wrong during the conversion process.

The unsorted MUA data per session by channel and trial are available at CRCNS. It would be useful for users to have this data available in DataJoint and the exported NWB files in case they want to re-spike-sort the data or re-segment the waveforms using other methods. Using low threshold multi-units may reveal interesting, different results compared with the manually sorted data.

Ideally there would be zero loss of information in the process of exporting to NWB. Animal strain and source are part of the Subject table in DataJoint but not stored in the NWB subject. Although NWB does not have a structured field for those data, please add them into the subject's description.

https://github.com/vathes/DJ-NWB-Li-Daie-2015-2016/blob/ae69335a23e6a3c0c3809a54de5480baf8848712/pipeline/export/datajoint_to_nwb.py#L65-L71

The early_lick field of a trial is stored as a string ("early" or "no early"), but would be more effectively stored as a boolean. In the NWB export, this would also allow early_lick to be stored as a boolean in the trials table.

https://github.com/vathes/DJ-NWB-Li-Daie-2015-2016/blob/ae69335a23e6a3c0c3809a54de5480baf8848712/pipeline/ingest/ingest_data_Li_2015.py#L118-L121

The EarlyLick schema says that there are three possible values, but the second value is never encountered in either of these datasets. Unless this schema is used for other datasets and there is a need to keep this consistent across datasets, I think EarlyLick should be a boolean instead of a varchar.

https://github.com/vathes/DJ-NWB-Li-Daie-2015-2016/blob/ae69335a23e6a3c0c3809a54de5480baf8848712/pipeline/experiment.py#L179-L189

The virus information is being converted from structured key-value pairs into JSON and then being stored as a string in NWBFile.virus.

https://github.com/vathes/DJ-NWB-Li-Daie-2015-2016/blob/ae69335a23e6a3c0c3809a54de5480baf8848712/pipeline/export/datajoint_to_nwb.py#L73-L74

For one file, this value is:

[{"injection_id": "1",
 "virus": "Addgene41015",
 "injection_date": "2013-05-23",
 "injection_volume": "30.0",
 "brain_location_name": "left_m2",
 "ml_location": "2500.0",
 "ap_location": "-1500.0",
 "dv_location": "500.0",
 "virus_source": "Janelia core",
 "virus_lot_number": "",
 "virus_titer": "None"},
 {"injection_id": "2",
 "virus": "Addgene41015",
 "injection_date": "2013-05-23",
 "injection_volume": "30.0",
 "brain_location_name": "left_m2",
 "ml_location": "2500.0",
 "ap_location": "-1500.0",
 "dv_location": "850.0",
 "virus_source": "Janelia core",
 "virus_lot_number": "",
 "virus_titer": "None"}]

I recommend making an extension to NWB that introduces a new data type called "Injections" that extends the DynamicTable type, and has "virus", "injection_date", "injection_volume", etc. as columns. Extensions to NWB for such structured information are useful for making the data machine-readable and encouraging a full description of the metadata. I can see this new data type being generally useful for storing virus data for many different datasets, so it would be good to create a single standard and use it across datasets. If the extension is popular or useful enough, then it would be considered for integration into the core NWB schema.

I would be happy to help you get started with creating and using an extension for virus injection metadata if you are interested in doing this.

Empty strings are used for required values here

https://github.com/vathes/DJ-NWB-Li-Daie-2015-2016/blob/2d74d3d7840f170187a4f063cbbbc22f991565ba/pipeline/export/datajoint_to_nwb.py#L56

and here

https://github.com/vathes/DJ-NWB-Li-Daie-2015-2016/blob/2d74d3d7840f170187a4f063cbbbc22f991565ba/pipeline/export/datajoint_to_nwb.py#L183

This is not good practice and is causing problems for the file to be read in matnwb.