Make VectorIndex extend VectorData

hdmf-dev / hdmf-common-schema

Specifications for pre-defined data structures provided by HDMF.

Other

3 stars 7 forks source link

Make VectorIndex extend VectorData #37

Closed rly closed 4 years ago

rly commented 4 years ago

Summary of changes

Resolve #32

PR Checklist

[x] Add release notes for the PR to docs/source/format_release_notes.rst

oruebel commented 4 years ago

If the Index type is unused, then we may want to remove it. I don't think anyone is using it so it should be save to remove. Is that correct @ajtritt

ajtritt commented 4 years ago

If the Index type is unused, then we may want to remove it.

Agreed. It's not used anywhere else.

rly commented 4 years ago

Fixed. I also removed: https://github.com/hdmf-dev/hdmf-common-schema/blob/9b159ef7d0989f7c92229b2a6c4c02c93ecee69b/common/table.yaml#L151-L153

which is now redundant.

oruebel commented 4 years ago

Fixed. I also removed:

I think it would be useful then to update the doc for DynamicTable to describe that VectorData may be indexed by VectroIndex to create ragged array columns and that the name of the VectorIndex is expected to be the same as the VectorData followed by the _index. The main reason is, that when removing VectorIndex as an explicit include the ability create ragged columns becomes a bit of "hidden" feature in the schema (even though formally nothing changes).

rly commented 4 years ago

I updated the docstrings for DynamicTable and VectorIndex. Please re-review

ajtritt commented 4 years ago

Looks good to me. Good luck with the API changes....