[Feature]: ImageSeries: check starting_frame when external_file is used

[weiglszonja]

What would you like to see added to PyNWB?

Related to #1318 and #1470, when external_file is used in ImageSeries a new check should be added which ensures that
a) starting_frame is provided if external_file is used,
b) starting_frame has the same length as external_file.

Currently starting_frame is set to [0] by default, which is fine for a single external file but is incorrect for multiple external files.

Is your feature request related to a problem?

No response

What solution would you like?

A new check method in ImageSeries.init that checks if
a) starting_frame is provided if external_file is used
b) starting_frame has the same length as external_file.

Do you have any interest in helping implement the feature?

Yes.

Code of Conduct

• I agree to follow this project's Code of Conduct
• Have you checked the Contributing document?
• Have you ensured this change was not already requested?

[weiglszonja]

Actually, there is a default value of [0] for starting_frame:

pynwb/src/pynwb/image.py

Lines 46 to 49 in 6505d86

	{'name': 'starting_frame', 'type': Iterable,
	'doc': 'Each entry is the frame number in the corresponding external_file variable. '
	'This serves as an index to what frames each file contains. If external_file is not '
	'provided, then this value will be None', 'default': [0]},

that gets overwritten when external file is not used:

pynwb/src/pynwb/image.py

Lines 77 to 78 in 6505d86

	if args_to_set["external_file"] is None:
	args_to_set["starting_frame"] = None # overwrite starting_frame

therefore a) is silently taken care of. do you think @oruebel this should be changed?
the second warning would get triggered with the default value as well if it doesn't match the length of external_file, I was just wondering if this logic should be changed.

[oruebel]

do you think @oruebel this should be changed?

In principle I think it should be sufficient in this case to just add the check to enforce that starting_frame has the same length as external_file.

On a separate note though, from a user perspective, I find this current behavior a bit confusing. I would have expected the behavior to be reversed. I.e.:

1. Have the default value for starting_frame as None
2. Set starting_frame to [0,] in __init__ if external_file has length 1

The end result of this is ultimately the same as the current behavior, so this is not critical. However, I find it strange that the default value of starting_frame reflects the very specific case of external_file with a single file, rather than the more general case of data being provided. I just think from a documentation perspective, having the default value of starting_frame be None would be more intuitive. What do you think?

[weiglszonja]

I completely agree, I would change it to None as well.

[oruebel]

I completely agree, I would change it to None as well.

Sounds good. In this case, I would suggest we:

1. Add the check to ensure that starting_frame has the same length as external_file
2. Change the default value for starting_frame to None
3. Set starting_frame to [0,] in __init__ if external_file has length 1

Item (3) is mainly for backwards compatibility as it ensures that the behavior of the API doesn't change in case that a user provides a single external_file, thus, avoiding a breaking change and allowing us to include this change in a minor or bugfix release, rather than requiring a major version bump.