Fix NWBData shadowing the data attribute it inherits from Data

[h-mayorquin]

Motivation

I found this while trying to get neuroconv's default chunking and gzip compression to apply to GrayscaleImage datasets, and discovering that it silently did nothing. NWBData re-declares self.__data in its __init__ and overrides the data property to read it, which shadows the storage it inherits from hdmf.container.Data. The inherited set_data_io writes the parent's attribute while .data reads the child's, so the DataIO wrapping is applied and then never seen again, and the dataset is written uncompressed with nothing warning you. This affects every NWBData subclass, so ScratchData, ImageReferences, ExternalImage and the image types, and the same shadow leaves transform, append, and extend operating on a different attribute than the parent thinks it is holding.

The fix is to let the parent own the storage. The private attribute and the data property go, __len__ was a duplicate of hdmf's so it goes with them, and append and extend become a type guard that delegates to super(), which keeps the existing error message for scalar data.

Two behavior changes worth a reviewer's attention. Data.__init__ runs coerce_pandas_data before storing while NWBData stashed the raw argument, so .data now returns the coerced value, meaning a pandas.Series passed as data reads back as the ndarray that was always the thing actually written to the file. And because append and extend now go through hdmf's append_data and extend_data, appending an array to ndarray-backed data concatenates instead of raising ValueError: all the input arrays must have same number of dimensions. That is the reproduction in #727, which this improves but does not close, since the list and ndarray cases still diverge. The existing test suite passes without modification.

How to test the behavior?

import numpy as np
from hdmf.backends.hdf5 import H5DataIO
from pynwb.image import GrayscaleImage

image = GrayscaleImage(name="image", data=np.zeros((4, 4)))
image.set_data_io(H5DataIO, dict(compression="gzip"))

# before: <class 'numpy.ndarray'>, the H5DataIO is stranded on the parent attribute
# after:  <class 'hdmf.backends.hdf5.h5_utils.H5DataIO'>
print(type(image.data))

Checklist

• Did you update CHANGELOG.md with your changes?
• Have you checked our Contributing document?
• Have you ensured the PR clearly describes the problem and the solution?
• Is your contribution compliant with our coding style? This can be checked running ruff check . && codespell from the source directory.
• Have you checked to ensure that there aren't other open Pull Requests for the same change?
• Have you included the relevant issue number using "Fix #XXX" notation where XXX is the issue number? By including "Fix #XXX" you allow GitHub to close issue #XXX when the PR is merged.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.22%. Comparing base (5942a7f) to head (ede92af).

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #2233      +/-   ##
==========================================
- Coverage   96.25%   96.22%   -0.04%     
==========================================
  Files          30       30              
  Lines        3018     2993      -25     
  Branches      438      433       -5     
==========================================
- Hits         2905     2880      -25     
  Misses         64       64              
  Partials       49       49              

Flag	Coverage Δ
integration	75.00% <ø> (+0.28%)	⬆️
unit	86.60% <ø> (-0.08%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[rly]

Good catch @h-mayorquin ! I think this is ultimately due to an incomplete refactor when HDMF was extracted out of PyNWB and NWBData began inheriting from hdmf's Data. Almost all contents of NWBData can be removed, except __init__ to keep the AllowPositional.WARNING. Could you remove them all and re-verify?

Could you also please simplify the changelog entry? The first sentence is sufficient.

[h-mayorquin]

Yes.

I modified the changelog as requested. It was the product of a verbose model in the top of a verbose person.

Thanks for the review.

[rly]

Thanks @h-mayorquin . What do you think about removing all contents of NWBData, except __init__ to keep the AllowPositional.WARNING? I'm happy to do it - let me know.

[h-mayorquin]

Should be done.

[rly]

Thank you @h-mayorquin !