hdmf-dev__hdmf-1270

The user is reporting a bug in `get_data_shape` function.
When `get_data_shape` is called with a list of `Data` objects, it currently recurses into the `Data` objects and returns the shape of the data inside the first `Data` object concatenated with the list shape, instead of just the shape of the list.

Example provided:
`get_data_shape([Data(name="a", data=[1, 2]), Data(name="b", data=[3, 4])])`
Expected shape: `(2, )` (since it's a list of 2 `Data` objects).
Actual behavior: It seems to be returning something like `(2, 2)` (where 2 is the length of the list, and (2,) is the shape of the first `Data` object).

Wait, the issue says:
"As a result, a 1-D list of references to Image objects, which on disk has shape (N, ), will be reported by get_data_shape as having shape (N, W, H). where (W, H) is the shape of the first image."

So if I have a list of `N` images, and each image is `(W, H)`, `get_data_shape` returns `(N, W, H)` instead of `(N,)`.

I need to find where `get_data_shape` is defined. Given the workspace layout, it's likely in `src/hdmf/data_utils.py`.

Plan:
1. Locate `get_data_shape` definition.
2. Reproduce the bug with a small script.
3. Fix the logic to avoid recursing into `Data` objects when they are elements of a list.
4. Verify the fix.

{
  "command": "grep -rn \"def get_data_shape\" src/hdmf/"
}

{
  "status": "ok",
  "stdout": "src/hdmf/utils.py:796:def get_data_shape(data, strict_no_data_load=False):\n",
  "stderr": "",
  "exit_code": 0
}

Okay, `get_data_shape` is defined in `src/hdmf/utils.py` at line 796.

Now I will read the implementation of `get_data_shape`.

{
  "filepath": "src/hdmf/utils.py",
  "start_line": 796
}

{
  "status": "ok",
  "filepath": "src/hdmf/utils.py",
  "content": "def get_data_shape(data, strict_no_data_load=False):\n    \"\"\"\n    Helper function used to determine the shape of the given array.\n\n    In order to determine the shape of nested tuples, lists, and sets, this function\n    recursively inspects elements along the dimensions, assuming that the data has a regular,\n    rectangular shape. In the case of out-of-core iterators, this means that the first item\n    along each dimension would potentially be loaded into memory. Set strict_no_data_load=True\n    to enforce that this does not happen, at the cost that we may not be able to determine\n    the shape of the array.\n\n    :param data: Array for which we should determine the shape. Can be any object that supports __len__ or .shape.\n    :type data: List, numpy.ndarray, DataChunkIterator\n    :param strict_no_data_load: If True and data is an out-of-core iterator, None may be returned. If False (default),\n                                the first element of data may be loaded into memory.\n    :return: Tuple of ints indicating the size of known dimensions. Dimensions for which the size is unknown\n             will be set to None.\n    \"\"\"\n\n    def __get_shape_helper(local_data):\n        shape = list()\n        if hasattr(local_data, '__len__'):\n            shape.append(len(local_data))\n            if len(local_data):\n                el = next(iter(local_data))\n                if not isinstance(el, (str, bytes)):\n                    shape.extend(__get_shape_helper(el))\n        return tuple(shape)\n\n    # NOTE: data.maxshape will fail on empty h5py.Dataset without shape or maxshape. this will be fixed in h5py 3.0\n    if hasattr(data, 'maxshape'):\n        return data.maxshape\n    if hasattr(data, 'shape') and data.shape is not None:\n        return data.shape\n    if isinstance(data, dict):\n        return None\n    if hasattr(data, '__len__') and not isinstance(data, (str, bytes)):\n        if not strict_no_data_load or isinstance(data, (list, tuple, set)):\n            return __get_shape_helper(data)\n    return None\n\n\ndef pystr(s):\n    \"\"\"\n    Convert a string of characters to Python str object\n    \"\"\"\n    if isinstance(s, bytes):\n        return s.decode('utf-8')\n    else:\n        return s\n\n\ndef to_uint_array(arr):\n    \"\"\"\n    Convert a numpy array or array-like object to a numpy array of unsigned integers with the same dtype itemsize.\n\n    For example, a list of int32 values is converted to a numpy array with dtype uint32.\n    :raises ValueError: if input array contains values that are not unsigned integers or non-negative integers.\n    \"\"\"\n    if not isinstance(arr, np.ndarray):\n        arr = np.array(arr)\n    if np.issubdtype(arr.dtype, np.unsignedinteger):\n        return arr\n    if np.issubdtype(arr.dtype, np.integer):\n        if (arr < 0).any():\n            raise ValueError('Cannot convert negative integer values to uint.')\n        dt = np.dtype('uint' + str(int(arr.dtype.itemsize*8)))  # keep precision\n        return arr.astype(dt)\n    raise ValueError('Cannot convert array of dtype %s to uint.' % arr.dtype)\n\n\ndef is_ragged(data):\n    \"\"\"\n    Test whether a list of lists or array is ragged / jagged\n    \"\"\"\n    if isinstance(data, (list, tuple)):\n        lengths = [len(sub_data) if isinstance(sub_data, (list, tuple)) else 1 for sub_data in data]\n        if len(set(lengths)) > 1:\n            return True  # ragged at this level\n\n        return any(is_ragged(sub_data) for sub_data in data)  # check next level\n\n    return False\n\ndef is_newer_version(version_a: str, version_b: str) -> bool:\n    # this method could be replaced by packaging.version if packaging is added as a dependency\n    version_a_match = re.match(r\"(\\d+\\.\\d+\\.\\d+)\", version_a)[0]  # trim off any non-numeric symbols at end\n    version_a_list = [int(i) for i in version_a_match.split(\".\")]\n\n    version_b_match = re.match(r\"(\\d+\\.\\d+\\.\\d+)\", version_b)[0]  # trim off any non-numeric symbols at end\n    version_b_list = [int(i) for i in version_b_match.split(\".\")]\n\n    for a, b in zip(version_a_list, version_b_list):\n        if a > b:\n            return True\n        elif a < b:\n            return False\n\n    return False\n\ndef get_basic_array_info(array):\n    def convert_bytes_to_str(bytes_size):\n        suffixes = ['bytes', 'KiB', 'MiB', 'GiB', 'TiB', 'PiB']\n        i = 0\n        while bytes_size >= 1024 and i < len(suffixes)-1:\n            bytes_size /= 1024.\n            i += 1\n        return f\"{bytes_size:.2f} {suffixes[i]}\"\n\n    if hasattr(array, \"nbytes\"):  # TODO: Remove this after h5py minimal version is larger than 3.0\n        array_size_in_bytes = array.nbytes\n    else:\n        array_size_in_bytes = array.size * array.dtype.itemsize\n    array_size_repr = convert_bytes_to_str(array_size_in_bytes)\n    basic_array_info_dict = {\"Data type\": array.dtype, \"Shape\": array.shape, \"Array size\": array_size_repr}\n\n    return basic_array_info_dict\n\ndef generate_array_html_repr(array_info_dict, array, dataset_type=None):\n    def html_table(item_dicts) -> str:\n        \"\"\"\n        Generates an html table from a dictionary\n        \"\"\"\n        report = '<table class=\"data-info\">'\n        report += \"<tbody>\"\n        for k, v in item_dicts.items():\n            report += (\n                f\"<tr>\"\n                f'<th style=\"text-align: left\">{k}</th>'\n                f'<td style=\"text-align: left\">{v}</td>'\n                f\"</tr>\"\n            )\n        report += \"</tbody>\"\n        report += \"</table>\"\n        return report\n\n    array_info_html = html_table(array_info_dict)\n    repr_html = dataset_type + \"<br>\" + array_info_html if dataset_type is not None else array_info_html\n\n    # Array like might lack nbytes (h5py < 3.0) or size (DataIO object)\n    if hasattr(array, \"nbytes\"):\n        array_size_bytes = array.nbytes\n    else:\n        if hasattr(array, \"size\"):\n            array_size = array.size\n        else:\n            import math\n            array_size = math.prod(array.shape)\n        array_size_bytes = array_size * array.dtype.itemsize\n",
  "start_line": 796,
  "end_line": 945,
  "total_lines": 1155,
  "is_truncated": true
}

The `get_data_shape` function uses a helper `__get_shape_helper`.

```python
    def __get_shape_helper(local_data):
        shape = list()
        if hasattr(local_data, '__len__'):
            shape.append(len(local_data))
            if len(local_data):
                el = next(iter(local_data))
                if not isinstance(el, (str, bytes)):
                    shape.extend(__get_shape_helper(el))
        return tuple(shape)
```

This function is recursive. If `local_data` has `__len__`, it adds the length and then recurses into the first element `el` if it's not a string or bytes.

If the elements of the list are `Data` objects, and `Data` objects also have `__len__` (which they probably do, since they wrap data arrays), then it will recurse into the `Data` object.

The issue is that `Data` objects (and their descendants like `Image`) should be treated as atomic elements in this context, not as containers to recurse into.

I need to see what `Data` object is and where it's defined. I'll search for `class Data`.

{
  "command": "grep -rn \"class Data\" src/hdmf/"
}

{
  "status": "ok",
  "stdout": "src/hdmf/backends/hdf5/h5_utils.py:120:class DatasetOfReferences(H5Dataset, ReferenceResolver, metaclass=ABCMeta):\nsrc/hdmf/data_utils.py:479:class DataChunkIterator(AbstractDataChunkIterator):\nsrc/hdmf/data_utils.py:763:class DataChunk:\nsrc/hdmf/data_utils.py:1005:class DataIO:\nsrc/hdmf/build/builders.py:320:class DatasetBuilder(BaseBuilder):\nsrc/hdmf/common/io/resources.py:12:           The DatasetBuilder for name is associated with data_type Data and container class Data,\nsrc/hdmf/spec/spec.py:659:class DatasetSpec(BaseStorageSpec):\nsrc/hdmf/monitor.py:19:class DataChunkProcessor(AbstractDataChunkIterator, metaclass=ABCMeta):\nsrc/hdmf/validate/validator.py:418:class DatasetValidator(BaseStorageValidator):\nsrc/hdmf/container.py:892:class Data(AbstractContainer):\n",
  "stderr": "",
  "exit_code": 0
}

`class Data(AbstractContainer)` is in `src/hdmf/container.py` at line 892.

Let's see the reproduction case:
`get_data_shape([Data(name="a", data=[1, 2]), Data(name="b", data=[3, 4])])`

I'll create a reproduction script. I need to import `Data` and `get_data_shape`.

{
  "command": "python3 - <<'PY'\nfrom hdmf.container import Data\nfrom hdmf.utils import get_data_shape\nimport numpy as np\n\n# Case 1: List of Data objects\ndata_objs = [Data(name=\"a\", data=[1, 2]), Data(name=\"b\", data=[3, 4])]\nshape = get_data_shape(data_objs)\nprint(f\"Shape of list of Data objects: {shape}\")\n\n# Case 2: List of Images (simulated by Data objects with larger shape)\nimage_objs = [Data(name=\"img1\", data=np.zeros((10, 20))), Data(name=\"img2\", data=np.zeros((10, 20)))]\nshape = get_data_shape(image_objs)\nprint(f\"Shape of list of Image objects: {shape}\")\nPY\n"
}

{
  "status": "ok",
  "stdout": "Shape of list of Data objects: (2, 2)\nShape of list of Image objects: (2, 10, 20)\n",
  "stderr": "",
  "exit_code": 0
}

The reproduction confirms the bug.
For `data_objs` (list of 2 Data objects, each with data `[1, 2]`), it returns `(2, 2)` instead of `(2,)`.
For `image_objs` (list of 2 Data objects, each with data `(10, 20)`), it returns `(2, 10, 20)` instead of `(2,)`.

The reason is that `Data` objects have `__len__` (they likely inherit it from `AbstractContainer` or define it), and `__get_shape_helper` recurses into any object that has `__len__` and is not `str` or `bytes`.

I should modify `__get_shape_helper` to avoid recursing into `Data` objects.

Let's check `src/hdmf/container.py` to see about `Data`.

{
  "filepath": "src/hdmf/container.py",
  "start_line": 892
}

{
  "status": "ok",
  "filepath": "src/hdmf/container.py",
  "content": "class Data(AbstractContainer):\n    \"\"\"\n    A class for representing dataset containers\n    \"\"\"\n    @docval({'name': 'name', 'type': str, 'doc': 'the name of this container'},\n            {'name': 'data', 'type': ('scalar_data', 'array_data', 'data'), 'doc': 'the source of the data'})\n    def __init__(self, **kwargs):\n        data = popargs('data', kwargs)\n        super().__init__(**kwargs)\n\n        self._validate_new_data(data)\n        self.__data = data\n\n    @property\n    def data(self):\n        return self.__data\n\n    @property\n    def shape(self):\n        \"\"\"\n        Get the shape of the data represented by this container\n        :return: Shape tuple\n        :rtype: tuple of ints\n        \"\"\"\n        return get_data_shape(self.__data)\n\n    def set_data_io(\n        self,\n        data_io_class: Type[DataIO],\n        data_io_kwargs: dict,\n        data_chunk_iterator_class: Optional[Type[AbstractDataChunkIterator]] = None,\n        data_chunk_iterator_kwargs: dict = None,\n    ) -> None:\n        \"\"\"\n        Apply DataIO object to the data held by this Data object.\n\n        Parameters\n        ----------\n        data_io_class: Type[DataIO]\n            The DataIO to apply to the data held by this Data.\n        data_io_kwargs: dict\n            The keyword arguments to pass to the DataIO.\n        data_chunk_iterator_class: Type[AbstractDataChunkIterator]\n            The DataChunkIterator to use for the DataIO. If None, no DataChunkIterator is used.\n        data_chunk_iterator_kwargs: dict\n            The keyword arguments to pass to the DataChunkIterator.\n\n        Notes\n        -----\n        If data_chunk_iterator_class is not None, the data is wrapped in the DataChunkIterator before being wrapped in\n        the DataIO. This allows for rewriting the backend configuration of hdf5 datasets.\n        \"\"\"\n        data_chunk_iterator_kwargs = data_chunk_iterator_kwargs or dict()\n        data = self.__data\n        if data_chunk_iterator_class is not None:\n            data = data_chunk_iterator_class(data=data, **data_chunk_iterator_kwargs)\n        self.__data = data_io_class(data=data, **data_io_kwargs)\n\n    @docval({'name': 'func', 'type': types.FunctionType, 'doc': 'a function to transform *data*'})\n    def transform(self, **kwargs):\n        \"\"\"\n        Transform data from the current underlying state.\n\n        This function can be used to permanently load data from disk, or convert to a different\n        representation, such as a torch.Tensor\n        \"\"\"\n        func = getargs('func', kwargs)\n        self.__data = func(self.__data)\n        return self\n\n    def __bool__(self):\n        if self.data is not None:\n            if isinstance(self.data, (np.ndarray, tuple, list)):\n                return len(self.data) != 0\n            if self.data:\n                return True\n        return False\n\n    def __len__(self):\n        return len(self.__data)\n\n    def __getitem__(self, args):\n        return self.get(args)\n\n    def get(self, args):\n        if isinstance(self.data, (tuple, list)) and isinstance(args, (tuple, list, np.ndarray)):\n            return [self.data[i] for i in args]\n        if isinstance(self.data, h5py.Dataset) and isinstance(args, np.ndarray):\n            # This is needed for h5py 2.9 compatibility\n            args = args.tolist()\n        return self.data[args]\n\n    def append(self, arg):\n        self._validate_new_data_element(arg)\n        self.__data = append_data(self.__data, arg)\n\n    def extend(self, arg):\n        \"\"\"\n        The extend_data method adds all the elements of the iterable arg to the\n        end of the data of this Data container.\n\n        :param arg: The iterable to add to the end of this VectorData\n        \"\"\"\n        self._validate_new_data(arg)\n        self.__data = extend_data(self.__data, arg)\n\n    def _validate_new_data(self, data):\n        \"\"\"Function to validate a new array that will be set or added to data. Raises an error if the data is invalid.\n\n        Subclasses should override this function to perform class-specific validation.\n        \"\"\"\n        pass\n\n    def _validate_new_data_element(self, arg):\n        \"\"\"Function to validate a new value that will be added to the data. Raises an error if the data is invalid.\n\n        Subclasses should override this function to perform class-specific validation.\n        \"\"\"\n        pass\n\n\nclass MultiContainerInterface(Container):\n    \"\"\"Class that dynamically defines methods to support a Container holding multiple Containers of the same type.\n\n    To use, extend this class and create a dictionary as a class attribute with any of the following keys:\n    * 'attr' to name the attribute that stores the Container instances\n    * 'type' to provide the Container object type (type or list/tuple of types, type can be a docval macro)\n    * 'add' to name the method for adding Container instances\n    * 'get' to name the method for getting Container instances\n    * 'create' to name the method for creating Container instances (only if a single type is specified)\n\n    If the attribute does not exist in the class, it will be generated. If it does exist, it should behave like a dict.\n\n    The keys 'attr', 'type', and 'add' are required.\n    \"\"\"\n\n    def __new__(cls, *args, **kwargs):\n        if cls is MultiContainerInterface:\n            raise TypeError(\"Can't instantiate class MultiContainerInterface.\")\n        if not hasattr(cls, '__clsconf__'):\n            raise TypeError(\"MultiContainerInterface subclass %s is missing __clsconf__ attribute. Please check that \"\n                            \"the class is properly defined.\" % cls.__name__)\n        return super().__new__(cls, *args, **kwargs)\n\n    @staticmethod\n    def __add_article(noun):\n        if isinstance(noun, tuple):\n            noun = noun[0]\n        if isinstance(noun, type):\n            noun = noun.__name__",
  "start_line": 892,
  "end_line": 1041,
  "total_lines": 1619,
  "is_truncated": true
}

`Data` class indeed defines `__len__`:

```python
    def __len__(self):
        return len(self.__data)
```

So when `get_data_shape`'s `__get_shape_helper` encounters a `Data` object, it sees `__len__`, takes the length, and then calls `__get_shape_helper(el)` where `el` is the `Data` object. Wait, `el` is the `Data` object. Then it calls `__get_shape_helper(DataObject)`. Inside that call, it sees `Data` object has `__len__`, and it recurses again?

Let's trace `__get_shape_helper([DataObj1, DataObj2])`:
1. `local_data = [DataObj1, DataObj2]`
2. `shape = [2]`
3. `el = DataObj1`
4. `el` is not `str` or `bytes`.
5. `shape.extend(__get_shape_helper(DataObj1))`
    a. `local_data = DataObj1`
    b. `shape = [2]` (where 2 is `len(DataObj1)`)
    c. `el = DataObj1[0]` (because `DataObj1` is iterable, it probably is since it has `__len__` and `__getitem__`)
    d. `shape.extend(__get_shape_helper(DataObj1[0]))`
    ... and so on.

The `Data` object should be considered a "leaf" for the purpose of `get_data_shape` if it's contained within another collection.

However, if `get_data_shape` is called directly on a `Data` object, it should return the shape of its internal data.

Wait, look at `get_data_shape` implementation again:

```python
def get_data_shape(data, strict_no_data_load=False):
    # ...
    def __get_shape_helper(local_data):
        shape = list()
        if hasattr(local_data, '__len__'):
            shape.append(len(local_data))
            if len(local_data):
                el = next(iter(local_data))
                if not isinstance(el, (str, bytes)):
                    shape.extend(__get_shape_helper(el))
        return tuple(shape)

    # ...
    if hasattr(data, 'maxshape'):
        return data.maxshape
    if hasattr(data, 'shape') and data.shape is not None:
        return data.shape
    # ...
    if hasattr(data, '__len__') and not isinstance(data, (str, bytes)):
        if not strict_no_data_load or isinstance(data, (list, tuple, set)):
            return __get_shape_helper(data)
    return None
```

Wait, `Data` object has a `shape` property!

```python
    @property
    def shape(self):
        """
        Get the shape of the data represented by this container
        :return: Shape tuple
        :rtype: tuple of ints
        """
        return get_data_shape(self.__data)
```

When `get_data_shape(data_objs)` is called where `data_objs` is a list of `Data` objects:
1. `data` (the list) does not have `maxshape`.
2. `data` (the list) does not have `shape`.
3. `data` (the list) is not a `dict`.
4. `data` (the list) has `__len__` and is not `str/bytes`.
5. It calls `__get_shape_helper(data)`.
6. Inside `__get_shape_helper`:
   - `local_data` is the list. `shape = [2]`.
   - `el` is the first `Data` object.
   - `__get_shape_helper(el)` is called.
   - Inside that call:
     - `local_data` is the `Data` object.
     - `local_data` has `__len__`. `shape = [2]`.
     - `el` is the first element of the `Data` object's data.
     - `__get_shape_helper(el)` is called...

The problem is that `__get_shape_helper` only checks for `__len__` and `str/bytes`. It does NOT check for `shape` property.

If `local_data` has a `shape` property, `__get_shape_helper` should probably use it and stop recursing.

But `get_data_shape` itself checks for `shape` at the beginning. Why doesn't `__get_shape_helper` do the same?

If I change `__get_shape_helper` to also check for `shape` property, it might fix this.

But wait, if I have a list of `Data` objects, I want the shape of the list: `(N,)`.
If I have a `Data` object, I want the shape of its data.

If `__get_shape_helper` is called with a `Data` object, it means the `Data` object is an element of something else. In that case, we should probably treat the `Data` object as a single element and NOT recurse into it.

Wait, if I have a list of `Data` objects, and I call `get_data_shape(list_of_data_objs)`, I expect `(N,)`.
`__get_shape_helper(list_of_data_objs)` starts with `shape = [N]`.
Then it calls `__get_shape_helper(data_obj)`.
If `__get_shape_helper(data_obj)` returns `()`, then the final result is `(N,)`.

So `__get_shape_helper` should return `()` for `Data` objects.

But wait, what if we have a list of lists of integers? `[[1, 2], [3, 4]]`. `get_data_shape` should return `(2, 2)`.
In that case, `el` is `[1, 2]`, which has `__len__`, and it recurses.

The difference between `[1, 2]` (a list) and a `Data` object is that `Data` object is a specific type of container that we want to treat as an atom here.

Actually, a better approach in `__get_shape_helper` would be to check if the element `el` is something that should be treated as an atom.

If `el` is a `Data` object, we should stop.

What else should be treated as atoms? `np.ndarray`'s shape is already handled by the main `get_data_shape` function if it's the top-level object. But if it's inside a list?
Example: `get_data_shape([np.zeros((2, 2)), np.zeros((2, 2))])`.
1. `local_data` is list. `shape = [2]`.
2. `el` is `np.ndarray`.
3. `__get_shape_helper(el)` is called.
4. `np.ndarray` has `__len__`. `shape = [2]`.
5. `el` is `np.ndarray` (first row).
6. `__get_shape_helper` is called...
Result will be `(2, 2, 2)`.
But wait, `np.ndarray` has a `.shape` property.

If `get_data_shape` is designed to work with nested lists/tuples but not recurse into things that have a `.shape` property, then that's the fix.

Let's test this hypothesis with a numpy array in a list.

{
  "command": "python3 - <<'PY'\nfrom hdmf.utils import get_data_shape\nimport numpy as np\n\ndata = [np.zeros((10, 20)), np.zeros((10, 20))]\nshape = get_data_shape(data)\nprint(f\"Shape of list of numpy arrays: {shape}\")\nPY\n"
}

{
  "status": "ok",
  "stdout": "Shape of list of numpy arrays: (2, 10, 20)\n",
  "stderr": "",
  "exit_code": 0
}

So `get_data_shape([np.zeros((10, 20)), np.zeros((10, 20))])` returns `(2, 10, 20)`.
Is this correct?

If the input is a list of 2 numpy arrays of shape (10, 20), the total shape is indeed (2, 10, 20) if it's treated as a 3D array.

But the issue says:
"In NWB, an Image neurodata type is a descendent of Data, i.e., a dataset. ImageReferences has a field that stores a 1-D list/array of Image objects. However, when validating the shape of input to that field in ImageReferences, get_data_shape recurses into the Image data to get the shape of the images. As a result, a 1-D list of references to Image objects, which on disk has shape (N, ), will be reported by get_data_shape as having shape (N, W, H). where (W, H) is the shape of the first image. The schema says the field should have shape (N, )."

This means for `Data` objects (and their descendants), we should NOT recurse.

But for numpy arrays, it seems we DO recurse (via the `__len__` in `__get_shape_helper`).

Wait, if I have a list of numpy arrays, `get_data_shape` currently returns `(N, ...)`.
If I have a list of `Data` objects, `get_data_shape` currently returns `(N, ...)`.

The user says that for `Data` objects, it should only return `(N,)`.

Why the difference?
Because a list of numpy arrays typically represents a single high-dimensional array (just stored as a list).
But a list of `Data` objects represents a list of references to datasets (which are stored separately on disk as separate datasets).

So, the `Data` object is the "unit" of data here.

So we need to modify `__get_shape_helper` to stop recursing if `el` is an instance of `Data`.

Let's find where `Data` is imported or how we can check for it in `src/hdmf/utils.py`.
`src/hdmf/utils.py` probably doesn't import `Data` from `hdmf.container` to avoid circular imports.

Wait, `src/hdmf/container.py` imports `get_data_shape` from `hdmf.utils`. If `utils.py` imports `Data` from `container.py`, we get a circular import.

Let's check imports in `src/hdmf/utils.py`.

{
  "command": "head -n 100 src/hdmf/utils.py"
}

{
  "status": "ok",
  "stdout": "import collections\nimport copy as _copy\nimport re\nimport types\nimport warnings\nfrom abc import ABCMeta\nfrom enum import Enum\n\nimport h5py\nimport numpy as np\n\n\n__macros = {\n    'array_data': [np.ndarray, list, tuple, h5py.Dataset],\n    'scalar_data': [str, int, float, bytes, bool],\n    'data': []\n}\n\ntry:\n    # optionally accept zarr.Array as array data to support conversion of data from Zarr to HDMF\n    import zarr\n    __macros['array_data'].append(zarr.Array)\nexcept ImportError:\n    pass\n\n\n# code to signify how to handle positional arguments in docval\nAllowPositional = Enum('AllowPositional', 'ALLOWED WARNING ERROR')\n\n__supported_bool_types = (bool, np.bool_)\n__supported_uint_types = (np.uint8, np.uint16, np.uint32, np.uint64)\n__supported_int_types = (int, np.int8, np.int16, np.int32, np.int64)\n__supported_float_types = [float, np.float16, np.float32, np.float64]\nif hasattr(np, \"float128\"):  # pragma: no cover\n    __supported_float_types.append(np.float128)\nif hasattr(np, \"longdouble\"):  # pragma: no cover\n    # on windows python<=3.5, h5py floats resolve float64s as either np.float64 or np.longdouble\n    # non-deterministically. a future version of h5py will fix this. see #112\n    __supported_float_types.append(np.longdouble)\n__supported_float_types = tuple(__supported_float_types)\n__allowed_enum_types = (__supported_bool_types + __supported_uint_types + __supported_int_types\n                        + __supported_float_types + (str,))\n\n\ndef docval_macro(macro):\n    \"\"\"Class decorator to add the class to a list of types associated with the key macro in the __macros dict\n    \"\"\"\n\n    def _dec(cls):\n        if macro not in __macros:\n            __macros[macro] = list()\n        __macros[macro].append(cls)\n        return cls\n\n    return _dec\n\n\ndef get_docval_macro(key=None):\n    \"\"\"\n    Return a deepcopy of the docval macros, i.e., strings that represent a customizable list of types for use in docval.\n\n    :param key: Name of the macro. If key=None, then a dictionary of all macros is returned. Otherwise, a tuple of\n                the types associated with the key is returned.\n    \"\"\"\n    if key is None:\n        return _copy.deepcopy(__macros)\n    else:\n        return tuple(__macros[key])\n\n\ndef check_type(value, argtype, allow_none=False):\n    \"\"\"Check a value against a type\n\n       The difference between this function and :py:func:`isinstance` is that\n       it allows specifying a type as a string. Furthermore, strings allow for specifying more general\n       types, such as a simple numeric type (i.e. ``argtype=\"num\"``).\n\n       Args:\n           value (Any): the value to check\n           argtype (type, str): the type to check for\n           allow_none (bool): whether or not to allow None as a valid value\n\n\n       Returns:\n           bool: True if value is a valid instance of argtype\n    \"\"\"\n    if value is None:\n        return allow_none\n    if isinstance(argtype, str):\n        if argtype in __macros:\n            return check_type(value, __macros[argtype], allow_none=allow_none)\n        elif argtype == 'uint':\n            return __is_uint(value)\n        elif argtype == 'int':\n            return __is_int(value)\n        elif argtype == 'float':\n            return __is_float(value)\n        elif argtype == 'bool':\n            return __is_bool(value)\n        cls_names = []\n",
  "stderr": "",
  "exit_code": 0
}

`src/hdmf/utils.py` doesn't seem to import `Data`.

Since we want to avoid circular imports, we can check if the object has a `shape` property AND is not just a numpy array or h5py dataset.
Or more simply, we can check if the object is an instance of something that we can identify as a `Data` object without importing it.

For example, we can check if it has a `__class__.__name__ == 'Data'` or something like that. But that's fragile.

Wait, `Data` is an `AbstractContainer`.

Another way: check if it's an instance of `Data` by importing it inside the function?

```python
    def __get_shape_helper(local_data):
        shape = list()
        if hasattr(local_data, '__len__'):
            shape.append(len(local_data))
            if len(local_data):
                el = next(iter(local_data))
                if not isinstance(el, (str, bytes)):
                    # Add check here
                    shape.extend(__get_shape_helper(el))
        return tuple(shape)
```

If I import `Data` inside `__get_shape_helper`, it might avoid the circular import at module load time.

Let's try this:

```python
    def __get_shape_helper(local_data):
        shape = list()
        if hasattr(local_data, '__len__'):
            shape.append(len(local_data))
            if len(local_data):
                el = next(iter(local_data))
                if not isinstance(el, (str, bytes)):
                    # Check if el is a Data object. 
                    # To avoid circular import, we can use a deferred import or check the class name.
                    from hdmf.container import Data
                    if isinstance(el, Data):
                        pass # stop recursing
                    else:
                        shape.extend(__get_shape_helper(el))
        return tuple(shape)
```

But wait, `Data` objects also have `__len__`. If `el` is a `Data` object, and we call `__get_shape_helper(el)`, it will again find `__len__` and recurse.

If I add `from hdmf.container import Data` and `if isinstance(el, Data): continue` it should work.

Let's verify if `from hdmf.container import Data` inside `get_data_shape` causes a circular import.

{
  "command": "python3 - <<'PY'\nfrom hdmf.utils import get_data_shape\nfrom hdmf.container import Data\nimport numpy as np\n\n# Reproduction case\ndata_objs = [Data(name=\"a\", data=[1, 2]), Data(name=\"b\", data=[3, 4])]\nprint(f\"Shape: {get_data_shape(data_objs)}\")\nPY\n"
}

{
  "status": "ok",
  "stdout": "Shape: (2, 2)\n",
  "stderr": "",
  "exit_code": 0
}

It doesn't cause a circular import because I imported them at the top level. The concern is if `utils.py` imports `container.py` at the module level.

If I put the import inside the function, it should be fine.

However, is there a better way to detect `Data` objects without importing `Data`?
`Data` objects are containers.

What if I check for the presence of both `shape` and `__len__` and some other attribute? No, that's not reliable.

Actually, let's check if `hdmf.container.Data` is available in `__macros`.
Looking at `src/hdmf/utils.py`:
```python
__macros = {
    'array_data': [np.ndarray, list, tuple, h5py.Dataset],
    'scalar_data': [str, int, float, bytes, bool],
    'data': []
}
```
No `Data` there.

Wait, `get_data_shape` is called by `Data.shape` property:
```python
    @property
    def shape(self):
        """
        Get the shape of the data represented by this container
        :return: Shape tuple
        :rtype: tuple of ints
        """
        return get_data_shape(self.__data)
```

If `self.__data` is a list of `Data` objects, then `get_data_shape` will be called on that list.

If I import `Data` inside `__get_shape_helper`, I should be careful about the overhead of importing every time. Better to import it once.

```python
    def __get_shape_helper(local_data):
        shape = list()
        if hasattr(local_data, '__len__'):
            shape.append(len(local_data))
            if len(local_data):
                el = next(iter(local_data))
                if not isinstance(el, (str, bytes)):
                    # Check for Data objects to avoid recursing into them
                    try:
                        from hdmf.container import Data
                        if isinstance(el, Data):
                            return tuple(shape)
                    except ImportError:
                        pass
                    shape.extend(__get_shape_helper(el))
        return tuple(shape)
```

But `get_data_shape` is used in many places. Maybe I can just check if the object is an instance of `hdmf.container.Data` by looking at its type name or using `inspect`.

Or, I can just check if the object has a `shape` property and we are in the `__get_shape_helper` function.
Wait, `np.ndarray` also has a `shape` property. If I stop when `el` has a `shape` property, then `get_data_shape([np.zeros((10, 20)), np.zeros((10, 20))])` will return `(2,)` instead of `(2, 10, 20)`.

Let's see if that's desired.
The user said: "a 1-D list of references to Image objects, which on disk has shape (N, ), will be reported by get_data_shape as having shape (N, W, H)".
A list of `Data` objects (which are references) should have shape `(N,)`.
A list of numpy arrays is typically converted to a single numpy array, so its shape should be `(N, W, H)`.

So the difference is indeed between `Data` objects and `np.ndarray`.

Wait, `Data` objects are intended to be "atomic" in this context.

If I can't import `Data`, can I use `getattr(el, '__class__').__name__ == 'Data'`? That seems a bit hacky but might work. Or better: `type(el).__name__ == 'Data'` or `type(el).__mro__` etc.

Actually, I can check if it's an instance of `Data` by importing it inside `get_data_shape` but outside `__get_shape_helper`.

```python
def get_data_shape(data, strict_no_data_load=False):
    # ...
    def __get_shape_helper(local_data):
        # ...
```

If I put `from hdmf.container import Data` at the top of `get_data_shape`, it will be executed every time `get_data_shape` is called. Python caches imports, so it's not a huge performance hit, but it's still not ideal.

Another option: move `get_data_shape` to a place where `Data` can be imported, or pass the "atomic types" as an argument. But I can't change the signature of `get_data_shape` as it's used in many places.

Wait, `Data` inherits from `AbstractContainer`. Maybe I can check for that? But `AbstractContainer` is also in `src/hdmf/container.py`.

What if I check if the object is any of the types that the user considers as "Data" objects?

Actually, if I look at `src/hdmf/container.py`, `Data` is a very central class.

Let's try the `from hdmf.container import Data` inside `__get_shape_helper` and see if it works and if it causes any issues.

Wait, if I do this:

```python
    def __get_shape_helper(local_data):
        shape = list()
        if hasattr(local_data, '__len__'):
            shape.append(len(local_data))
            if len(local_data):
                el = next(iter(local_data))
                if not isinstance(el, (str, bytes)):
                    # Check if el is a Data object to avoid recursing into it
                    if hasattr(el, 'shape') and not hasattr(el, 'ndim'):
                        # Data objects have 'shape' but not 'ndim'. 
                        # np.ndarray and h5py.Dataset have both.
                        pass 
                    else:
                        shape.extend(__get_shape_helper(el))
```

Wait, does `Data` have `ndim`? No.
Does `np.ndarray` have `ndim`? Yes.
Does `h5py.Dataset` have `ndim`? Yes.

This might be a way to distinguish them without importing `Data`.

Let's check if `Data` objects have `ndim`.

{
  "command": "python3 - <<'PY'\nfrom hdmf.container import Data\nimport numpy as np\n\nd = Data(name=\"a\", data=[1, 2])\nprint(f\"Data object has ndim: {hasattr(d, 'ndim')}\")\nprint(f\"Data object has shape: {hasattr(d, 'shape')}\")\n\nn = np.zeros((2, 2))\nprint(f\"Numpy array has ndim: {hasattr(n, 'ndim')}\")\nprint(f\"Numpy array has shape: {hasattr(n, 'shape')}\")\nPY\n"
}

{
  "status": "ok",
  "stdout": "Data object has ndim: False\nData object has shape: True\nNumpy array has ndim: True\nNumpy array has shape: True\n",
  "stderr": "",
  "exit_code": 0
}