NeurodataWithoutBorders / matnwb

A Matlab interface for reading and writing NWB files
BSD 2-Clause "Simplified" License
49 stars 32 forks source link

smarter reading of VectorIndex #115

Closed bendichter closed 3 years ago

bendichter commented 5 years ago

We could use a more convenient way to read the corresponding data for specific row(s) of indexed vectors. We have util.read_indexed_column, which works OK, but it would be better to have this feature as a method of VectorIndex, similar to the refresh and deref methods.

lawrence-mbf commented 5 years ago

It will be difficult to just add methods to VectorIndex as it is a generated class. The one way to get around this would be to add a hardcoded override for metaClass but I'm not a big fan of that. If we implement the dynamic table -> nwb table functions then wouldn't it also solve the issue?

bendichter commented 5 years ago

This would address the particular case when you want to read a portion of an indexed vector. table -> nwb DynamicTable wouldn't address this because that is a write function, not a read function. I don't think nwb DynamicTable -> table would address this either because I was imagining that function would read the entire nwb DynamicTable. If we can't attach methods, then maybe the util function is the best we can do for now.

lawrence-mbf commented 5 years ago

Could be a case for https://github.com/NeurodataWithoutBorders/matnwb/issues/98#issuecomment-467636351 as well.

bendichter commented 5 years ago

Yes, it seems like it may be another good case for it.

lawrence-mbf commented 3 years ago

@bendichter The new getRow function in DynamicTable allows you to grab DynamicTable data given a valid id. Are there any other ways that users index into the table that would be helpful?

bendichter commented 3 years ago

@ln-vidrio yes, by row number is equally common. They are often the same but not always, because users are allowed to set their own ids.

lawrence-mbf commented 3 years ago

Oh, this is a good point. Right now addRow actually requires the ids to be auto-incremented I think. Should the id be required to be set and we just check for duplicates?

bendichter commented 3 years ago

In PyNWB most users do not define the ids themselves but allow them to be populated automatically. There is the option of adding ids manually, which is generally used when a user wants the ids to be consistent with another data source. Would that be possible here as well?

lawrence-mbf commented 3 years ago

I'd have to add that in I think. At that point, is the only requirement that it the id is unique or is that optional as well?

bendichter commented 3 years ago

It needs to be both. I'm not 100% sure that we confirm it is unique if it is input manually on the PyNWB side. Maybe it is checked on write

oruebel commented 3 years ago

I'm not 100% sure that we confirm it is unique

If I remember correctly this can be configured in the PyNWB API, i.e., you can ask it to enforce unique IDs (which I believe some types do) but in general the id's are not checked for uniqueness. In most cases I'd expect the id's to be unique though since folks typically let the API set the id's automatically, in which case PyNWB uses row-index as id.

lawrence-mbf commented 3 years ago

Okay, I can leave regular getRow before to get the row by index, then add a keyword argument allowing for an id to query by id instead, which supercedes whatever the index argument is.