Clarify intended behavior of ImageSeries.num_samples for internal (non-external) data

[ehennestad]

Clarification request: ImageSeries.num_samples is persisted for internal (non-external) data

I'd like to understand the intended behavior of num_samples on ImageSeries, because the inherited in-memory TimeSeries.num_samples property interacts with the schema-defined num_samples dataset (added in #2194) in a way that seems inconsistent with the field's documented meaning. I'm raising this as a question rather than a bug report — I may be misreading the intent.

Background

• TimeSeries.num_samples is a computed property that returns len(data) (falling back to len(timestamps)).

• ImageSeries overrides this property, and Add num_samples to ImageSeries #2194 also added num_samples as an actual schema dataset (nwb.image.yaml), documented as:

  Total number of frames across all external files. This is required when format='external' and timing is described using starting_time and rate, since data is empty and its first dimension cannot be used to determine the number of frames. When timestamps is provided, len(timestamps) already serves this purpose.

The doc frames num_samples as an external-file concept: a way to recover a frame count that cannot otherwise be derived. The _check_num_samples warning is likewise gated on external_file is not None and rate is not None and num_samples is None.

Observed behavior

For an internal ImageSeries (data stored in the file, no external file), the user never sets num_samples, but:

1. The overridden property falls through to TimeSeries.num_samples and returns len(data).
2. There is no custom IO mapper, so the default ObjectMapper persists whatever the property returns — so num_samples gets written to disk even though data.shape[0] already carries that information.

import numpy as np
from pynwb.image import ImageSeries

img = ImageSeries(name="img", data=np.zeros((10, 5, 5)), unit="n.a.", rate=1.0)
print(img._num_samples)  # None  -- user never set it
print(img.num_samples)   # 10    -- inherited TimeSeries property returns len(data)
# on write -> acquisition/img/num_samples == 10 is persisted to the file

So the persisted num_samples value on an internal ImageSeries doesn't correspond to the documented meaning of the field ("total number of frames across all external files") — there are no external files. The value is numerically correct, but it's a redundant write of a field the schema doc describes as external-only.

Questions

1. Is persisting num_samples for internal (non-external) ImageSeries intended, or should the dataset only be written in the external-file case it was designed for?
2. If it's not intended, would the preferred fix be to decouple the in-memory convenience property from persistence (e.g. only write the dataset when external_file is set / data is empty), so that ImageSeries.num_samples can still return the frame count in Python without writing a redundant/misapplied dataset?
3. Should the schema doc be broadened to also describe the internal-data meaning, if persisting it there is in fact intended?

Environment

• pynwb 4.0.0
• hdmf 6.1.0
• Python 3.11.7

🤖 Generated with Claude Code

[rly]

Good questions and good catch.

ImageSeries.num_samples for reference: https://github.com/NeurodataWithoutBorders/nwb-schema/blob/a92016a6f7e938d5bfbebac675fd993c46c2ee5c/core/nwb.image.yaml#L105-L111

I would say:

1. Persisting num_samples for internal (non-external) ImageSeries is an oversight. I don't think the dataset should be written when it is not explicitly provided. It would not be so bad if it were, but if we do that, we should be consistent across the schema. Currently ImageSeries.dimension is optional and could also be computed from internal data.
   • I would add that if num_samples is explicitly provided for an internal dataset, we should check that it is consistent with the number of frames in the dataset.
2. Yes
3. We should clarify the schema doc to say that if data is internal and this field is supplied, it should be equal to the number of frames.

Alternatively, we could change the usage of num_samples in the schema to be much more restrictive and normalized: num_samples should be supplied only if format='external' and timing is described using starting_time and rate. But I think that it doesn't hurt to have that information as a convenience, as long as it is consistent when present.

Curious to hear others' thoughts @oruebel @bendichter @h-mayorquin

[h-mayorquin]

I think the cheapest thing here is just to modify the docs so num_samples applies to any user of ImageSeries, and keep the behavior as-is.

The persisted value is always correct (for internal data it equals data.shape[0]), so the only real issue is that the doc frames the field as external-only, which makes the write look misapplied. If we broaden it to describe the total frame count (required for the external + starting_time/rate case, and for internal data optional but equal to data.shape[0] when present), the value matches its documentation and the field stays a harmless convenience in the internal case, which is itself an edge case.

I'd rather not decouple the property from persistence. That is pynwb IO work plus a consistency check, it pulls in the dimension question, and it makes the field's presence depend on the pynwb version. I don't think is worth it.

[ehennestad]

I think the cheapest thing here is just to modify the docs so num_samples applies to any user of ImageSeries, and keep the behavior as-is.

I agree.

As a general note though, this expands on a concept which the schema currently can not not express: dependent values (i.e a value that should be computed from other groups or datasets). Those already existing which I am aware of is colnames (DynamicTable) and categories (AlignedDynamicTable).

Since the schema does not have a way to express/enforce dependent values, the responsibility lies on the API (i.e PyNWB or MatNWB) or the users themselves to ensure that it is correct. From a user perspective, I think it is better that the API handles this. But two problems with that:

1. There is no mechanism for an API maintainer to discover such dependencies, except for reading doc strings.
2. For a fully generated api, like MatNWB, the enforcement requires a custom hook. This is an extra (though minor) maintenance cost.

[oruebel]

Those already existing which I am aware of is colnames (DynamicTable) and categories (AlignedDynamicTable).

Just for additional clarification, colnames (and categories) exist to allow a user to define the order in which columns should appear (e.g., when converting to a pandas DataFrame in Python). As such, while an API can compute a default value based on the order in which a user added the columns; it is ultimately up to the user to define the order in which columns should appear.

As such, these are "dependent values" only in the sense that the validity of the content can/should be enforced (i.e., colnames must contain the names of all columns), however, the exact value (here the specific order of columns) cannot be prescribed because the ordering is up to the user. Also, the specific column names are not known in the schema, since this depends on which columns the user creates when they write a file. This makes specifying these constraints in the schema tricky.

• There is no mechanism for an API maintainer to discover such dependencies, except for reading doc strings.

• For a fully generated api, like MatNWB, the enforcement requires a custom hook. This is an extra (though minor) maintenance cost

Yes, that is correct.