NeurodataWithoutBorders / matnwb

A Matlab interface for reading and writing NWB files
BSD 2-Clause "Simplified" License
49 stars 32 forks source link

Add explicit checks to DataStub indices #514

Closed lawrence-mbf closed 10 months ago

lawrence-mbf commented 11 months ago

fix #513

Motivation

The original code probably assumed that the low-level HDF5 calls would check for valid inputs and spaces. This appears not to be the case for the following two cases:

1) If a non-scalar index exceeds the bounds of a DataStub, it repeats the values of the first (valid) index instead. 2) In certain cases, using a scalar index that exceeds the bounds of the DataStub returns an empty cell array.

How to test the behavior?

This example script sets up a condition where issue (1) occurs ```MATLAB filename = 'test.h5'; fullDatasetPath = '/data'; datasetName = 'data'; dataSize = 10; if 2 == exist(filename, 'file') delete(filename); end h5create(filename, fullDatasetPath, dataSize); h5write(filename, fullDatasetPath, 1:10); h5disp(filename); DataStub = types.untyped.DataStub(filename, fullDatasetPath); DataStub([1, 11]) % returns [1;1] for some reason % DataStub(11) % throws an error, complicates ticket #513 ```

As mentioned in the code snippet, (2) is currently not reproducible. However, this PR should resolve both potential index errors anyway.

Checklist

codecov[bot] commented 11 months ago

Codecov Report

Merging #514 (e05ab5b) into master (92a65a3) will increase coverage by 0.03%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #514      +/-   ##
==========================================
+ Coverage   87.89%   87.92%   +0.03%     
==========================================
  Files         129      129              
  Lines        5451     5468      +17     
==========================================
+ Hits         4791     4808      +17     
  Misses        660      660              
Impacted Files Coverage Δ
+types/+untyped/DataStub.m 95.41% <100.00%> (+0.34%) :arrow_up:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more