add io attr (#882)

* add io attr

* Add read_io as a property on AbstractContainer

* Fix ruff

* Fix existing tests

* Added tests for AbstractContainer.read_io property

* Updated changelog

* Attempt to fix failing Winodws tests

* Attempt to fix failing Winodws tests

* Attempt to fix failing Winodws tests

* Attempt to fix failing Winodws tests

* Attempt to fix failing Winodws tests

---------

Co-authored-by: Oliver Ruebel <[email protected]>
Co-authored-by: Oliver Ruebel <[email protected]>
Co-authored-by: Ryan Ly <[email protected]>

Author: 3 people

CHANGELOG.md

@@ -7,6 +7,7 @@
 - Added abstract static method `HDMFIO.can_read()` and concrete static method `HDF5IO.can_read()`. @bendichter [#875](https://github.com/hdmf-dev/hdmf/pull/875)
 - Added warning for `DynamicTableRegion` links that are not added to the same parent as the original container object. @mavaylon1 [#891](https://github.com/hdmf-dev/hdmf/pull/891)
 - Added the `TermSet` class along with integrated validation methods for any child of `AbstractContainer`, e.g., `VectorData`, `Data`, `DynamicTable`. @mavaylon1 [#880](https://github.com/hdmf-dev/hdmf/pull/880)
+- Added `AbstractContainer.read_io` property to be able to retrieve the HDMFIO object used for reading from the container and to ensure the I/O object used for reading is not garbage collected before the container is being deleted. @bendichter @oruebel [#882](https://github.com/hdmf-dev/hdmf/pull/882)
 - Allow for `datetime.date` to be used instead of `datetime.datetime`. @bendichter [#874](https://github.com/hdmf-dev/hdmf/pull/874)
 - Updated `HDMFIO` and `HDF5IO` to support `ExternalResources`. @mavaylon1 [#895](https://github.com/hdmf-dev/hdmf/pull/895)
 - Dropped Python 3.7 support. @rly [#897](https://github.com/hdmf-dev/hdmf/pull/897)

src/hdmf/backends/io.py

@@ -58,6 +58,7 @@ def read(self, **kwargs):
             # TODO also check that the keys are appropriate. print a better error message
             raise UnsupportedOperation('Cannot build data. There are no values.')
         container = self.__manager.construct(f_builder)
+        container.read_io = self
         if self.external_resources_path is not None:
             from hdmf.common import ExternalResources
             try:

src/hdmf/container.py

@@ -192,6 +192,11 @@ def __gather_fields(cls, name, bases, classdict):
         cls._set_fields(tuple(field_conf['name'] for field_conf in all_fields_conf))
         cls.__fieldsconf = tuple(all_fields_conf)
 
+    def __del__(self):
+        # Make sure the reference counter for our read IO is being decremented
+        del self.__read_io
+        self.__read_io = None
+
     def __new__(cls, *args, **kwargs):
         """
         Static method of the object class called by Python to create the object first and then
@@ -221,6 +226,56 @@ def __init__(self, **kwargs):
             raise ValueError("name '" + name + "' cannot contain '/'")
         self.__name = name
         self.__field_values = dict()
+        self.__read_io = None
+
+    @property
+    def read_io(self):
+        """
+        The  :class:`~hdmf.backends.io.HDMFIO` object used for reading the container.
+
+        This property will typically be  None if this Container is not a root Container
+        (i.e., if `parent` is not None). Use `get_read_io` instead if you want to retrieve the
+        :class:`~hdmf.backends.io.HDMFIO` object used for reading from the parent container.
+        """
+        return self.__read_io
+
+    @read_io.setter
+    def read_io(self, value):
+        """
+        Set the io object used to read this container
+
+        :param value: The :class:`~hdmf.backends.io.HDMFIO` object to use
+        :raises ValueError: If io has already been set. We can't change the IO for a container.
+        :raises TypeError: If value is not an instance of :class:`~hdmf.backends.io.HDMFIO`
+        """
+        # We do not want to import HDMFIO on the module level to avoid circular imports. Since we only need
+        # it for type checking we import it here.
+        from hdmf.backends.io import HDMFIO
+        if not isinstance(value, HDMFIO):
+            raise TypeError("io must be an instance of HDMFIO")
+        if self.__read_io is not None:
+            raise ValueError("io has already been set for this container (name=%s, type=%s)" %
+                             (self.name, str(type(self))))
+        else:
+            self.__read_io = value
+
+    def get_read_io(self):
+        """
+        Get the io object used to read this container.
+
+        If `self.read_io` is None, this function will iterate through the parents and return the
+        first `io` object found on a parent container
+
+        :returns: The :class:`~hdmf.backends.io.HDMFIO`  object used to read this container.
+                  Returns None in case no io object is found, e.g., in case this container has
+                  not been read from file.
+        """
+        curr_obj = self
+        re_io = self.read_io
+        while re_io is None and curr_obj.parent is not None:
+            curr_obj = curr_obj.parent
+            re_io = curr_obj.read_io
+        return re_io
 
     @property
     def name(self):

tests/unit/test_container.py

@@ -1,5 +1,6 @@
 import numpy as np
 from uuid import uuid4, UUID
+import os
 
 from hdmf.container import AbstractContainer, Container, Data, ExternalResourcesManager
 from hdmf.common.resources import ExternalResources
@@ -8,6 +9,7 @@
 from hdmf.common import (DynamicTable, VectorData, DynamicTableRegion)
 import unittest
 from hdmf.term_set import TermSet
+from hdmf.backends.hdf5.h5tools import HDF5IO
 
 try:
     import linkml_runtime  # noqa: F401
@@ -41,6 +43,13 @@ def test_link_and_get_resources(self):
 
 class TestContainer(TestCase):
 
+    def setUp(self):
+        self.path = "test_container.h5"
+
+    def tearDown(self):
+        if os.path.exists(self.path):
+            os.remove(self.path)
+
     def test_new(self):
         """Test that __new__ properly sets parent and other fields.
         """
@@ -82,6 +91,45 @@ def test_init(self):
         self.assertEqual(obj.children, tuple())
         self.assertIsNone(obj.parent)
         self.assertEqual(obj.name, 'obj1')
+        self.assertIsNone(obj.read_io)
+
+    def test_read_io_none(self):
+        """Test that __init__ properly sets read_io to None"""
+        obj = Container('obj1')
+        self.assertIsNone(obj.read_io)
+
+    def test_read_io_setter(self):
+        """Test setting the read IO property"""
+        obj = Container('obj1')
+        # Bad value for read_io
+        with self.assertRaises(TypeError):
+            obj.read_io = "test"
+        # Set read_io
+        with  HDF5IO(self.path, mode='w') as temp_io:
+            obj.read_io = temp_io
+            self.assertIs(obj.read_io, temp_io)
+            # Check that setting read_io again fails
+            with self.assertRaises(ValueError):
+                obj.read_io = temp_io
+
+    def test_get_read_io_on_self(self):
+        """Test that get_read_io works when the container is set on the container"""
+        obj = Container('obj1')
+        self.assertIsNone(obj.get_read_io())
+        with HDF5IO(self.path, mode='w') as temp_io:
+            obj.read_io = temp_io
+            re_io = obj.get_read_io()
+            self.assertIs(re_io, temp_io)
+
+    def test_get_read_io_on_parent(self):
+        """Test that get_read_io works when the container is set on the parent"""
+        parent_obj = Container('obj1')
+        child_obj = Container('obj2')
+        child_obj.parent = parent_obj
+        with HDF5IO(self.path, mode='w') as temp_io:
+            parent_obj.read_io = temp_io
+            self.assertIsNone(child_obj.read_io)
+            self.assertIs(child_obj.get_read_io(), temp_io)
 
     def test_set_parent(self):
         """Test that parent setter properly sets parent
@@ -481,7 +529,7 @@ class EmptyFields(AbstractContainer):
         self.assertTupleEqual(EmptyFields.get_fields_conf(), tuple())
 
         props = TestAbstractContainerFieldsConf.find_all_properties(EmptyFields)
-        expected = ['children', 'container_source', 'fields', 'modified', 'name', 'object_id', 'parent']
+        expected = ['children', 'container_source', 'fields', 'modified', 'name', 'object_id', 'parent', 'read_io']
         self.assertListEqual(props, expected)
 
     def test_named_fields(self):
@@ -502,7 +550,7 @@ def __init__(self, **kwargs):
 
         props = TestAbstractContainerFieldsConf.find_all_properties(NamedFields)
         expected = ['children', 'container_source', 'field1', 'field2', 'fields', 'modified', 'name', 'object_id',
-                    'parent']
+                    'parent', 'read_io']
         self.assertListEqual(props, expected)
 
         f1_doc = getattr(NamedFields, 'field1').__doc__
@@ -583,7 +631,7 @@ class NamedFieldsChild(NamedFields):
 
         props = TestAbstractContainerFieldsConf.find_all_properties(NamedFieldsChild)
         expected = ['children', 'container_source', 'field1', 'field2', 'fields', 'modified', 'name', 'object_id',
-                    'parent']
+                    'parent', 'read_io']
         self.assertListEqual(props, expected)
 
     def test_inheritance_override(self):