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

Make `beadl_taks_program` parameter optional #7

Closed oruebel closed 2 years ago

oruebel commented 2 years ago

Currently the EventTypesTable, StateTypesTable and ActionTypesTable require the beadl_task_program to be set on __init__ even if populare_from_program is to False. To allow the creation of these data structures without a program (e.g., as is the case in src/pynwb/tests/test_example.py) a user should be able to pass None for the beadl_task_program.

mavaylon1 commented 2 years ago

I'm not sure if we should. These tables are meant to reflect the program, and having manual additions would lead to errors in the validator if the user makes a mistake. I'm in favor of strictly enforcing automation with a program.

oruebel commented 2 years ago

These tables are meant to reflect the program

As long as the default is that a program is required, I think this is fine. I.e., as long as the user has to make the explicit choice to set the program to None, rather than None being the default value.

Also, I would envision that these data structures will be used also outside of just BEADL. I.e,. the only part that is specific to BEADL right now is the BEADLTaskProgram part, everything else is useful also beyond BEADL as the source for the data.

Along, the same lines, I think the beadl_task_program parameter should be replaced to just requiring a general task_program.

mavaylon1 commented 2 years ago

I made the changes so it not required anymore; however, I think we should keep it "beadl_task_program" till further development. I am not confident in this version being general enough for all task_programs

oruebel commented 2 years ago

however, I think we should keep it "beadl_task_program" till further development

Sounds good. In this case I suggest creating an issue ticket for future reference just so we have an open record as we move closer to release. At latest when we move towards integration with NWB, this is an issue that we should address. More broadly, I think this means that we should define what the interface for the generic TaskProgram class needs to look for it to work with the tables.