Skip to content

fix: export scalar datetime as a scalar dataset#835

Open
ehennestad wants to merge 3 commits into
mainfrom
fix-scalar-datetime-export
Open

fix: export scalar datetime as a scalar dataset#835
ehennestad wants to merge 3 commits into
mainfrom
fix-scalar-datetime-export

Conversation

@ehennestad

@ehennestad ehennestad commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator

Motivation

Problem

When a scalar datetime property such as session_start_time is set, types.util.correctType ran num2cell on it, converting the scalar datetime into a 1×1 cell array before formatting. On export that 1×1 cell was written to HDF5 as a 1-element, non-scalar dataset instead of a scalar dataset.

So a value that is conceptually scalar was stored with the wrong dataspace. The exported file no longer reflected the field's scalar shape, the value did not round-trip faithfully (a scalar went in, a 1-element array came back), and on read it was returned as a lazy DataStub instead of an eagerly-loaded scalar.

Solution

Drop the num2cell conversion and format datetimes in place, so the datetime branch of correctType preserves the class and shape of its input:

  • a scalar datetime stays a scalar datetime
  • a datetime array stays a datetime array (formatted element-wise)
  • a cell array of datetimes stays a cell array of datetimes

Scalar datetime properties are now written as scalar datasets. Covered by a test asserting the scalar dataspace on export, plus an updated reader test reflecting that a scalar dataset is read eagerly rather than as a DataStub.

How to test the behavior?

% session_start_time is a scalar datetime
nwb = tests.factory.NWBFile();
nwbExport(nwb, "scalar_datetime.nwb");

info = h5info("scalar_datetime.nwb", "/session_start_time");
disp(info.Dataspace.Type)   % "scalar"  (previously a 1-element, non-scalar dataset)

Checklist

  • Have you ensured the PR description clearly describes the problem and solutions?
  • Have you checked to ensure that there aren't other open or previously closed Pull Requests for the same change?
  • If this PR fixes an issue, is the first line of the PR description fix #XX where XX is the issue number?

🤖 Generated with Claude Code

- A scalar datetime was wrapped in a cell. This is an unnecessary mutation which is removed here
- A cell array of datetimes where all values either have TimeZone info or do not, is now output as a datetime array.
Undo non-scalar cell array to datetime array. Mutating outputs should be minimised
Add test ensuring scalar dataset is NOT read as datastub
Add test checking that scalar datetime is exported to a scalar dataspace
@ehennestad ehennestad requested a review from bendichter June 24, 2026 21:27
@ehennestad ehennestad enabled auto-merge June 24, 2026 21:27
@codecov

codecov Bot commented Jun 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.31%. Comparing base (6f4e97a) to head (f953063).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #835   +/-   ##
=======================================
  Coverage   95.31%   95.31%           
=======================================
  Files         219      219           
  Lines        7647     7647           
=======================================
  Hits         7289     7289           
  Misses        358      358           

☔ 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.

@ehennestad ehennestad added the category: bug errors in the code or code behavior label Jun 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category: bug errors in the code or code behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant