catalystneuro / buffalo-lab-to-nwb

Scripts to convert Buffalo Lab data to the NWB standard
2 stars 0 forks source link

GUI timestamps flag does not appear to do anything #23

Open bendichter opened 4 years ago

bendichter commented 4 years ago

In the conversion GUI, for SpatialSeries objects (and I'm guessing others), there is a "timestamps" flag that indicates whether or not to get timestamps from the source file. However in add_behavior.py, the SpatialSeries objects appear to load timestamps whether or not that flag is selected.

https://github.com/ben-dichter-consulting/buffalo-lab-data-to-nwb/blob/2ae387a236e0117e38d1d0c29a375c186bb9325a/src/buffalonwb/add_behavior.py#L34-L51

Let's aim to not have any controls that are not hooked up to the conversion code. It would be cool to allow the user to enter a starting time and sampling rate instead of getting the timestamps from the file, but I don't think that's critical here. If we are not using those flags, then let's remove them.

luiztauffer commented 4 years ago

For the GUI I set some fields as checkboxes to indicate whether that info should be extracted or not, those are usually datasets. I removed it already for data since it is always required, as you pointed out. You’re right that it could be a control to choose between timestamps or t0 + rate. I would need to make a radiobox to toggle between these options in the GUI. But maybe you would like to always enforce the nwb best practices given the available data in the source files, then giving this choice to the user wouldn’t make sense.

I could do either:

  1. create radiobox to toggle between the two options and add use this info in the conversion function.
  2. remove these checkboxes options altogether
bendichter commented 4 years ago

I'm fine with either