search

rly / ndx-structured-behavior

An NWB extension for storing structured behavior programs and data, such as from BAABL/BEADL
BSD 3-Clause "New" or "Revised" License
3 stars 3 forks source link

Duration Column for EventsTable and ActionsTable #37

Closed mavaylon1 closed 10 months ago

mavaylon1 commented 1 year ago

This adds a duration column (optional) to support how some data acquisition systems organize data as events/actions with durations. Beadl does this with states but this allows for 1) it implies that events/actions are instantaneous in nature, i.e., the state machine responds when the event/action happens, and keep events/actions distinct from states, 2) having duration as an optional column is more intuitive then having a start_time with an optional stop_time.

mavaylon1 commented 1 year ago

Fix #27

mavaylon1 commented 1 year ago

@oruebel This is a working draft PR for the issue ticket mentioned in the paper that I said I'd take care of before I leave. I will update the release notes as a pseudo changelog later and ping for review monday.

rly commented 10 months ago

Note that this PR removes TaskRecording. I vaguely remember that was because the EventsTable, StatesTable, and ActionsTable would go under acquisition or processing. @mavaylon1 @oruebel do you remember?

oruebel commented 10 months ago

Note that this PR removes TaskRecording

I do not recall why TaskRecording is being removed here. We currently mention it in the paper, but I can remove it if necessary. What makes me suspecious is that PR https://github.com/rly/ndx-structured-behavior/pull/36 that adds TaskRecording was merged a day after this PR was created, so I'm wondering whether that removing it may have been a mistake.

oruebel commented 10 months ago

Also note that the tutorial notebook is still using TaskRecording and would need to be updated. There also seem to be some other errors in the notebook (some errors and warnings from running the notebook) that got merged.

https://github.com/rly/ndx-structured-behavior/blob/main/docs/tutorials/babl_example_notebook.ipynb

oruebel commented 10 months ago

Storing the EventsTable, ActionsTable, and StatesTable in acquistion is fine with me, but we should definitely fix the tutorial notebook

rly commented 10 months ago

Good sleuthing. Perhaps it was removed from this branch to separate the PRs and get the other one merged, but because it was deleted in this branch, after the main branch was merged into this, git understands the deletion as intended. I will add it back.

oruebel commented 10 months ago

I will add it back.

Sounds good. Thanks! Can you please also rerun the notebook to check that it works.

mavaylon1 commented 10 months ago

@oruebel @rly Ryan is correct in that it was separated into a different PR.