Discuss use of columns in DynamicTable and ElectrodeTable

[oruebel]

I have a couple of questions about how DynamicTable.add_column works. I'm creating a separate issue for this here to not hold up merging #1

Q1 colNames is defined as a private attribute in both DynamicTable and in ElectrodeTable. It seems strange that sub-classes of DynamicTable would need to define their own private member colNames. I would suggest making colNames a protected member instead and then setting the default value for colNames = {"group", "group_name", "location"} should be set in the constructor of ElectrodeTable instead.

https://github.com/lbl-cbg/aq-nwb/blob/61eb390ec400102485e06a46d32212603cd78414/src/hdmf/table/DynamicTable.hpp#L92-L95

https://github.com/lbl-cbg/aq-nwb/blob/61eb390ec400102485e06a46d32212603cd78414/src/file/ElectrodeTable.hpp#L108-L111

Q2 It is unclear to me if and where DynamicTable tracks the columns that have been added to the table? If I understand the class correctly, it seems to currently only track the names of the columns but not the columns itself.

Q3 Related to Q2, it seems that ElectrodeTable tracks the data for the pre-defined columns via explicit variables. To allow for dynamic addition of columns and to make interacting with columns easier, I think it would be useful to have a mechanism to manage the columns. One approach maybe to use an std::unordered_map<std::string, VectorData> columns as a protected attribute on DynamicTable to keep track of all columns.

https://github.com/lbl-cbg/aq-nwb/blob/61eb390ec400102485e06a46d32212603cd78414/src/file/ElectrodeTable.hpp#L89-L108

Q4 This version of DynamicTable::addColumn appears to create multiple columns. Is this for convenience or should we have the function only do a single column or rename to addColumn

https://github.com/lbl-cbg/aq-nwb/blob/61eb390ec400102485e06a46d32212603cd78414/src/hdmf/table/DynamicTable.cpp#L29-L44

Q5 Also let's discuss how we want to set default columns. See https://github.com/lbl-cbg/aq-nwb/pull/1/files#r1542138441