Warn when adding ragged arrays to DynamicTable without index argument

[stephprince]

Motivation

Fix #1065. When a user tries to add a ragged array to a DynamicTable using add_row or add_column, a warning will occur prompting the user to use the index argument.

I had some questions about these potential changes.

1. I found a related issue discussion that suggested that doing a full inspection of the data to detect whether it was ragged might be a costly operation - Added unit_columns items are saved as strings NeurodataWithoutBorders/pynwb#1060. I was wondering if that's still a concern here or not.
2. Numpy's warning refers to the requested array having an "inhomogeneous shape". I wasn't sure if that type of phrasing is preferred over "ragged."

How to test the behavior?

from pynwb import NWBHDF5IO
from pynwb.testing.mock.file import mock_NWBFile

# test adding individual trials
nwbfile = mock_NWBFile()
nwbfile.add_trial_column(name="test", description="")
nwbfile.add_trial(start_time=0.0, stop_time=2.0, test=[1, 2])
nwbfile.add_trial(start_time=0.0, stop_time=2.0, test=[3, 4])
nwbfile.add_trial(start_time=0.0, stop_time=2.0, test=[5, 6, 7])  # Warning here

### test adding full column
nwbfile = mock_NWBFile()
nwbfile.add_trial(start_time=0.0, stop_time=2.0)
nwbfile.add_trial(start_time=0.0, stop_time=2.0)
nwbfile.add_trial_column(name="test1", description="", data=[[1, 2], [1, 2]])
nwbfile.add_trial_column(name="test2", description="", data=[[1, 2], [1, 2, 3]])  # Warning here

with NWBHDF5IO('test.nwb', 'w') as f:
    f.write(nwbfile)  # Error here

Checklist

• Did you update CHANGELOG.md with your changes?
• Does the PR clearly describe the problem and the solution?
• Have you reviewed our Contributing Guide?
• Does the PR use "Fix #XXX" notation to tell GitHub to close the relevant issue numbered XXX when the PR is merged?

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.55%. Comparing base (e9c2737) to head (a44206d).
Report is 56 commits behind head on dev.

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #1066      +/-   ##
==========================================
+ Coverage   88.53%   88.55%   +0.01%     
==========================================
  Files          45       45              
  Lines        9605     9616      +11     
  Branches     2732     2738       +6     
==========================================
+ Hits         8504     8515      +11     
  Misses        778      778              
  Partials      323      323              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[rly]

• I found a related issue discussion that suggested that doing a full inspection of the data to detect whether it was ragged might be a costly operation - Added unit_columns items are saved as strings NeurodataWithoutBorders/pynwb#1060. I was wondering if that's still a concern here or not.

Yes, please correct me if I am wrong - it looks like we are iterating over every element in the array to check for raggedness. That could incur performance issues. I think we should add a flag check_ragged (default True) so that a user can disable the check if it is causing issues.

I don't know how we would get around checking every element...

Note that this issue should only come up if 1) the data is a list or tuple (arrays and hdf5 datasets cannot be ragged unless you give it an object type) or 2) the user adds rows one by one to the table and a cell in that row has a different shape than the shapes of the cell in the previous rows. (the original post mentioned using this method)

Related to 1), I suggest limiting the is_ragged check in add_column to just list and tuple.

Related to 2), it looks like adding a check for add_row is not implemented. I think that would be good to do.

• Numpy's warning refers to the requested array having an "inhomogeneous shape". I wasn't sure if that type of phrasing is preferred over "ragged."

It seems like numpy uses "ragged" internally. I wonder why they chose to use "inhomogeneous shape" publicly. Perhaps "inhomogeneous shape" is more clear to the non-programmer, but I feel like both terms are poor. Maybe we could say "data has elements that have different lengths and therefore data cannot be coerced into an N-dimensional array"?

[stephprince]

I think we should add a flag check_ragged (default True) so that a user can disable the check if it is causing issues.

Sounds like a good option!

Note that this issue should only come up if 1) the data is a list or tuple (arrays and hdf5 datasets cannot be ragged unless you give it an object type)

Yes I wasn't sure how often people added arrays that were an object type but since it would still error when writing I think limiting this check make sense.

Related to 2), it looks like adding a check for add_row is not implemented. I think that would be good to do.

I had a check in add_row that inspects the whole column for raggedness every time a row was added, or do you mean additional tests for adding a row?

Perhaps "inhomogeneous shape" is more clear to the non-programmer, but I feel like both terms are poor.

I agree, I think the phrasing you suggested is good.

[rly]

Looks great!

[rly]

I had a check in add_row that inspects the whole column for raggedness every time a row was added

Ah, I missed that.