Skip to content

Commit

Permalink
Merge branch 'dev' into 1148_maxshape_property_H5DataIO
Browse files Browse the repository at this point in the history
  • Loading branch information
cboulay authored Aug 27, 2024
2 parents 993ffdf + abb6fe5 commit 0ec4167
Show file tree
Hide file tree
Showing 19 changed files with 346 additions and 56 deletions.
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ repos:
# hooks:
# - id: black
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.5.7
rev: v0.6.1
hooks:
- id: ruff
# - repo: https://github.com/econchick/interrogate
Expand Down
11 changes: 10 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,9 +1,16 @@
# HDMF Changelog

## HDMF 3.14.4 (Upcoming)
## HDMF 3.14.4 (August 22, 2024)

### Enhancements
- Added support to append to a dataset of references for HDMF-Zarr. @mavaylon1 [#1157](https://github.com/hdmf-dev/hdmf/pull/1157)
- Adjusted stacklevel of warnings to point to user code when possible. @rly [#1166](https://github.com/hdmf-dev/hdmf/pull/1166)
- Improved "already exists" error message when adding a container to a `MultiContainerInterface`. @rly [#1165](https://github.com/hdmf-dev/hdmf/pull/1165)
- Added support to write multidimensional string arrays. @stephprince [#1173](https://github.com/hdmf-dev/hdmf/pull/1173)
- Add support for appending to a dataset of references. @mavaylon1 [#1135](https://github.com/hdmf-dev/hdmf/pull/1135)

### Bug fixes
- Fixed issue where scalar datasets with a compound data type were being written as non-scalar datasets @stephprince [#1176](https://github.com/hdmf-dev/hdmf/pull/1176)

### Bug fixes
- Fix H5DataIO not exposing `maxshape` on non-dci dsets. @cboulay [#1149](https://github.com/hdmf-dev/hdmf/pull/1149)
Expand All @@ -14,6 +21,8 @@
- Added new attribute "dimension_labels" on `DatasetBuilder` which specifies the names of the dimensions used in the
dataset based on the shape of the dataset data and the dimension names in the spec for the data type. This attribute
is available on build (during the write process), but not on read of a dataset from a file. @rly [#1081](https://github.com/hdmf-dev/hdmf/pull/1081)
- Speed up loading namespaces by skipping register_type when already registered. @magland [#1102](https://github.com/hdmf-dev/hdmf/pull/1102)
- Speed up namespace loading: return a shallow copy rather than a deep copy in build_const_args. @magland [#1103](https://github.com/hdmf-dev/hdmf/pull/1103)

## HDMF 3.14.2 (July 7, 2024)

Expand Down
2 changes: 1 addition & 1 deletion docs/source/install_developers.rst
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ environment by using the ``conda remove --name hdmf-venv --all`` command.
For advanced users, we recommend using Mambaforge_, a faster version of the conda package manager
that includes conda-forge as a default channel.

.. _Anaconda: https://www.anaconda.com/products/distribution
.. _Anaconda: https://www.anaconda.com/download
.. _Mambaforge: https://github.com/conda-forge/miniforge

Install from GitHub
Expand Down
2 changes: 1 addition & 1 deletion docs/source/install_users.rst
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,4 @@ You can also install HDMF using ``conda`` by running the following command in a
conda install -c conda-forge hdmf
.. _Anaconda Distribution: https://www.anaconda.com/products/distribution
.. _Anaconda Distribution: https://www.anaconda.com/download
24 changes: 19 additions & 5 deletions src/hdmf/backends/hdf5/h5_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import logging

from ...array import Array
from ...data_utils import DataIO, AbstractDataChunkIterator
from ...data_utils import DataIO, AbstractDataChunkIterator, append_data
from ...query import HDMFDataset, ReferenceResolver, ContainerResolver, BuilderResolver
from ...region import RegionSlicer
from ...spec import SpecWriter, SpecReader
Expand Down Expand Up @@ -108,6 +108,20 @@ def ref(self):
def shape(self):
return self.dataset.shape

def append(self, arg):
# Get Builder
builder = self.io.manager.get_builder(arg)
if builder is None:
raise ValueError(
"The container being appended to the dataset has not yet been built. "
"Please write the container to the file, then open the modified file, and "
"append the read container to the dataset."
)

# Get HDF5 Reference
ref = self.io._create_ref(builder)
append_data(self.dataset, ref)


class DatasetOfReferences(H5Dataset, ReferenceResolver, metaclass=ABCMeta):
"""
Expand Down Expand Up @@ -501,7 +515,7 @@ def __init__(self, **kwargs):
# Check for possible collision with other parameters
if not isinstance(getargs('data', kwargs), Dataset) and self.__link_data:
self.__link_data = False
warnings.warn('link_data parameter in H5DataIO will be ignored', stacklevel=2)
warnings.warn('link_data parameter in H5DataIO will be ignored', stacklevel=3)
# Call the super constructor and consume the data parameter
super().__init__(**kwargs)
# Construct the dict with the io args, ignoring all options that were set to None
Expand All @@ -525,7 +539,7 @@ def __init__(self, **kwargs):
self.__iosettings.pop('compression', None)
if 'compression_opts' in self.__iosettings:
warnings.warn('Compression disabled by compression=False setting. ' +
'compression_opts parameter will, therefore, be ignored.', stacklevel=2)
'compression_opts parameter will, therefore, be ignored.', stacklevel=3)
self.__iosettings.pop('compression_opts', None)
# Validate the compression options used
self._check_compression_options()
Expand All @@ -540,7 +554,7 @@ def __init__(self, **kwargs):
if isinstance(self.data, Dataset):
for k in self.__iosettings.keys():
warnings.warn("%s in H5DataIO will be ignored with H5DataIO.data being an HDF5 dataset" % k,
stacklevel=2)
stacklevel=3)

self.__dataset = None

Expand Down Expand Up @@ -618,7 +632,7 @@ def _check_compression_options(self):
if self.__iosettings['compression'] not in ['gzip', h5py_filters.h5z.FILTER_DEFLATE]:
warnings.warn(str(self.__iosettings['compression']) + " compression may not be available "
"on all installations of HDF5. Use of gzip is recommended to ensure portability of "
"the generated HDF5 files.", stacklevel=3)
"the generated HDF5 files.", stacklevel=4)

@staticmethod
def filter_available(filter, allow_plugin_filters):
Expand Down
17 changes: 15 additions & 2 deletions src/hdmf/backends/hdf5/h5tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ def copy_file(self, **kwargs):
warnings.warn("The copy_file class method is no longer supported and may be removed in a future version of "
"HDMF. Please use the export method or h5py.File.copy method instead.",
category=DeprecationWarning,
stacklevel=2)
stacklevel=3)

source_filename, dest_filename, expand_external, expand_refs, expand_soft = getargs('source_filename',
'dest_filename',
Expand Down Expand Up @@ -698,6 +698,8 @@ def __read_dataset(self, h5obj, name=None):
d = ReferenceBuilder(target_builder)
kwargs['data'] = d
kwargs['dtype'] = d.dtype
elif h5obj.dtype.kind == 'V': # scalar compound data type
kwargs['data'] = np.array(scalar, dtype=h5obj.dtype)
else:
kwargs["data"] = scalar
else:
Expand Down Expand Up @@ -1227,6 +1229,8 @@ def _filler():

return
# If the compound data type contains only regular data (i.e., no references) then we can write it as usual
elif len(np.shape(data)) == 0:
dset = self.__scalar_fill__(parent, name, data, options)
else:
dset = self.__list_fill__(parent, name, data, options)
# Write a dataset containing references, i.e., a region or object reference.
Expand Down Expand Up @@ -1469,7 +1473,7 @@ def __list_fill__(cls, parent, name, data, options=None):
data_shape = io_settings.pop('shape')
elif hasattr(data, 'shape'):
data_shape = data.shape
elif isinstance(dtype, np.dtype):
elif isinstance(dtype, np.dtype) and len(dtype) > 1: # check if compound dtype
data_shape = (len(data),)
else:
data_shape = get_data_shape(data)
Expand Down Expand Up @@ -1514,6 +1518,7 @@ def __get_ref(self, **kwargs):
self.logger.debug("Getting reference for %s '%s'" % (container.__class__.__name__, container.name))
builder = self.manager.build(container)
path = self.__get_path(builder)

self.logger.debug("Getting reference at path '%s'" % path)
if isinstance(container, RegionBuilder):
region = container.region
Expand All @@ -1525,6 +1530,14 @@ def __get_ref(self, **kwargs):
else:
return self.__file[path].ref

@docval({'name': 'container', 'type': (Builder, Container, ReferenceBuilder), 'doc': 'the object to reference',
'default': None},
{'name': 'region', 'type': (slice, list, tuple), 'doc': 'the region reference indexing object',
'default': None},
returns='the reference', rtype=Reference)
def _create_ref(self, **kwargs):
return self.__get_ref(**kwargs)

def __is_ref(self, dtype):
if isinstance(dtype, DtypeSpec):
return self.__is_ref(dtype.dtype)
Expand Down
16 changes: 14 additions & 2 deletions src/hdmf/build/objectmapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,11 @@
from .errors import (BuildError, OrphanContainerBuildError, ReferenceTargetNotBuiltError, ContainerConfigurationError,
ConstructError)
from .manager import Proxy, BuildManager

from .warnings import (MissingRequiredBuildWarning, DtypeConversionWarning, IncorrectQuantityBuildWarning,
IncorrectDatasetShapeBuildWarning)
from hdmf.backends.hdf5.h5_utils import H5DataIO

from ..container import AbstractContainer, Data, DataRegion
from ..term_set import TermSetWrapper
from ..data_utils import DataIO, AbstractDataChunkIterator
Expand Down Expand Up @@ -598,11 +601,17 @@ def __get_data_type(cls, spec):

def __convert_string(self, value, spec):
"""Convert string types to the specified dtype."""
def __apply_string_type(value, string_type):
if isinstance(value, (list, tuple, np.ndarray, DataIO)):
return [__apply_string_type(item, string_type) for item in value]
else:
return string_type(value)

ret = value
if isinstance(spec, AttributeSpec):
if 'text' in spec.dtype:
if spec.shape is not None or spec.dims is not None:
ret = list(map(str, value))
ret = __apply_string_type(value, str)
else:
ret = str(value)
elif isinstance(spec, DatasetSpec):
Expand All @@ -618,7 +627,7 @@ def string_type(x):
return x.isoformat() # method works for both date and datetime
if string_type is not None:
if spec.shape is not None or spec.dims is not None:
ret = list(map(string_type, value))
ret = __apply_string_type(value, string_type)
else:
ret = string_type(value)
# copy over any I/O parameters if they were specified
Expand Down Expand Up @@ -972,6 +981,9 @@ def __get_ref_builder(self, builder, dtype, shape, container, build_manager):
for d in container.data:
target_builder = self.__get_target_builder(d, build_manager, builder)
bldr_data.append(ReferenceBuilder(target_builder))
if isinstance(container.data, H5DataIO):
# This is here to support appending a dataset of references.
bldr_data = H5DataIO(bldr_data, **container.data.get_io_params())
else:
self.logger.debug("Setting %s '%s' data to reference builder"
% (builder.__class__.__name__, builder.name))
Expand Down
2 changes: 1 addition & 1 deletion src/hdmf/common/resources.py
Original file line number Diff line number Diff line change
Expand Up @@ -628,7 +628,7 @@ def add_ref(self, **kwargs):
if entity_uri is not None:
entity_uri = entity.entity_uri
msg = 'This entity already exists. Ignoring new entity uri'
warn(msg, stacklevel=2)
warn(msg, stacklevel=3)

#################
# Validate Object
Expand Down
10 changes: 5 additions & 5 deletions src/hdmf/common/table.py
Original file line number Diff line number Diff line change
Expand Up @@ -717,7 +717,7 @@ def add_row(self, **kwargs):
warn(("Data has elements with different lengths and therefore cannot be coerced into an "
"N-dimensional array. Use the 'index' argument when creating a column to add rows "
"with different lengths."),
stacklevel=2)
stacklevel=3)

def __eq__(self, other):
"""Compare if the two DynamicTables contain the same data.
Expand Down Expand Up @@ -776,7 +776,7 @@ def add_column(self, **kwargs): # noqa: C901

if isinstance(index, VectorIndex):
warn("Passing a VectorIndex in for index may lead to unexpected behavior. This functionality will be "
"deprecated in a future version of HDMF.", category=FutureWarning, stacklevel=2)
"deprecated in a future version of HDMF.", category=FutureWarning, stacklevel=3)

if name in self.__colids: # column has already been added
msg = "column '%s' already exists in %s '%s'" % (name, self.__class__.__name__, self.name)
Expand All @@ -793,7 +793,7 @@ def add_column(self, **kwargs): # noqa: C901
"Please ensure the new column complies with the spec. "
"This will raise an error in a future version of HDMF."
% (name, self.__class__.__name__, spec_table))
warn(msg, stacklevel=2)
warn(msg, stacklevel=3)

index_bool = index or not isinstance(index, bool)
spec_index = self.__uninit_cols[name].get('index', False)
Expand All @@ -803,7 +803,7 @@ def add_column(self, **kwargs): # noqa: C901
"Please ensure the new column complies with the spec. "
"This will raise an error in a future version of HDMF."
% (name, self.__class__.__name__, spec_index))
warn(msg, stacklevel=2)
warn(msg, stacklevel=3)

spec_col_cls = self.__uninit_cols[name].get('class', VectorData)
if col_cls != spec_col_cls:
Expand Down Expand Up @@ -841,7 +841,7 @@ def add_column(self, **kwargs): # noqa: C901
warn(("Data has elements with different lengths and therefore cannot be coerced into an "
"N-dimensional array. Use the 'index' argument when adding a column of data with "
"different lengths."),
stacklevel=2)
stacklevel=3)

# Check that we are asked to create an index
if (isinstance(index, bool) or isinstance(index, int)) and index > 0 and len(data) > 0:
Expand Down
12 changes: 5 additions & 7 deletions src/hdmf/container.py
Original file line number Diff line number Diff line change
Expand Up @@ -629,12 +629,8 @@ def __repr__(self):
template += "\nFields:\n"
for k in sorted(self.fields): # sorted to enable tests
v = self.fields[k]
# if isinstance(v, DataIO) or not hasattr(v, '__len__') or len(v) > 0:
if hasattr(v, '__len__'):
if isinstance(v, (np.ndarray, list, tuple)):
if len(v) > 0:
template += " {}: {}\n".format(k, self.__smart_str(v, 1))
elif v:
if isinstance(v, (np.ndarray, list, tuple)) or v:
template += " {}: {}\n".format(k, self.__smart_str(v, 1))
else:
template += " {}: {}\n".format(k, v)
Expand Down Expand Up @@ -894,7 +890,7 @@ def set_dataio(self, **kwargs):
warn(
"Data.set_dataio() is deprecated. Please use Data.set_data_io() instead.",
DeprecationWarning,
stacklevel=2,
stacklevel=3,
)
dataio = getargs('dataio', kwargs)
dataio.data = self.__data
Expand Down Expand Up @@ -1142,7 +1138,9 @@ def _func(self, **kwargs):
# still need to mark self as modified
self.set_modified()
if tmp.name in d:
msg = "'%s' already exists in %s '%s'" % (tmp.name, cls.__name__, self.name)
msg = (f"Cannot add {tmp.__class__} '{tmp.name}' at 0x{id(tmp)} to dict attribute '{attr_name}' in "
f"{cls} '{self.name}'. {d[tmp.name].__class__} '{tmp.name}' at 0x{id(d[tmp.name])} "
f"already exists in '{attr_name}' and has the same name.")
raise ValueError(msg)
d[tmp.name] = tmp
return container
Expand Down
6 changes: 6 additions & 0 deletions src/hdmf/query.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,12 @@ def __next__(self):
def next(self):
return self.dataset.next()

def append(self, arg):
"""
Override this method to support appending to backend-specific datasets
"""
pass # pragma: no cover


class ReferenceResolver(metaclass=ABCMeta):
"""
Expand Down
28 changes: 16 additions & 12 deletions src/hdmf/spec/namespace.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,13 @@ def __init__(self, **kwargs):
self['full_name'] = full_name
if version == str(SpecNamespace.UNVERSIONED):
# the unversioned version may be written to file as a string and read from file as a string
warn("Loaded namespace '%s' is unversioned. Please notify the extension author." % name, stacklevel=2)
warn(f"Loaded namespace '{name}' is unversioned. Please notify the extension author.")
version = SpecNamespace.UNVERSIONED
if version is None:
# version is required on write -- see YAMLSpecWriter.write_namespace -- but can be None on read in order to
# be able to read older files with extensions that are missing the version key.
warn(("Loaded namespace '%s' is missing the required key 'version'. Version will be set to '%s'. "
"Please notify the extension author.") % (name, SpecNamespace.UNVERSIONED), stacklevel=2)
warn(f"Loaded namespace '{name}' is missing the required key 'version'. Version will be set to "
f"'{SpecNamespace.UNVERSIONED}'. Please notify the extension author.")
version = SpecNamespace.UNVERSIONED
self['version'] = version
if date is not None:
Expand Down Expand Up @@ -466,15 +466,19 @@ def __load_namespace(self, namespace, reader, resolve=True):
return included_types

def __register_type(self, ndt, inc_ns, catalog, registered_types):
spec = inc_ns.get_spec(ndt)
spec_file = inc_ns.catalog.get_spec_source_file(ndt)
self.__register_dependent_types(spec, inc_ns, catalog, registered_types)
if isinstance(spec, DatasetSpec):
built_spec = self.dataset_spec_cls.build_spec(spec)
if ndt in registered_types:
# already registered
pass
else:
built_spec = self.group_spec_cls.build_spec(spec)
registered_types.add(ndt)
catalog.register_spec(built_spec, spec_file)
spec = inc_ns.get_spec(ndt)
spec_file = inc_ns.catalog.get_spec_source_file(ndt)
self.__register_dependent_types(spec, inc_ns, catalog, registered_types)
if isinstance(spec, DatasetSpec):
built_spec = self.dataset_spec_cls.build_spec(spec)
else:
built_spec = self.group_spec_cls.build_spec(spec)
registered_types.add(ndt)
catalog.register_spec(built_spec, spec_file)

def __register_dependent_types(self, spec, inc_ns, catalog, registered_types):
"""Ensure that classes for all types used by this type are registered
Expand Down Expand Up @@ -529,7 +533,7 @@ def load_namespaces(self, **kwargs):
if ns['version'] != self.__namespaces.get(ns['name'])['version']:
# warn if the cached namespace differs from the already loaded namespace
warn("Ignoring cached namespace '%s' version %s because version %s is already loaded."
% (ns['name'], ns['version'], self.__namespaces.get(ns['name'])['version']), stacklevel=2)
% (ns['name'], ns['version'], self.__namespaces.get(ns['name'])['version']))
else:
to_load.append(ns)
# now load specs into namespace
Expand Down
Loading

0 comments on commit 0ec4167

Please sign in to comment.