pynwb icon indicating copy to clipboard operation
pynwb copied to clipboard

[Feature]: Raise error for `SpatialSeries` created by user with >3 columns

Open rly opened this issue 2 years ago • 1 comments

What would you like to see added to PyNWB?

See #1479. It is currently difficult to raise this error without making older files that contain a SpatialSeries with >3 columns unreadable, so a stopgap was implemented in #1480.

Is your feature request related to a problem?

No response

What solution would you like?

  1. Raise a warning when reading a file (so that files remain readable)
  2. Raise an error when a user creates a new SpatialSeries to prevent creation of new bad data.

Do you have any interest in helping implement the feature?

Yes.

Code of Conduct

rly avatar May 19 '22 22:05 rly

One solution may be to:

  • Have a flag __warn_on_invalid_data on the SpatialSeries.__init__ with the default being set to False
  • Have the ObjectMapper for SpatialSeries return True for the __warn_on_invalid_data
  • Have SpatialSeries.__init__ issue a warning if __warn_on_invalid_data is True and otherwise raise and exception

In this way, the default behavior for the user would be to raise and Exception when creating a new SpatialSeries and on read, when then Container class is constructed by the BuildManager using the ObjectMapper, the behavior would be to always just warn. The advantage of this approach is that it is fairly simple and implements the desired behavior. The disadvantage is that it adds a constructor argument for SpatialSeries so a user can still disable the behavior with they want to. Ideally, the flag to indicate whether to raise a warning or error should be hidden from the user.

Taking this approach a bit further, we could expand this to the Container base class and have essentially a mode flag to indicate whether a container is created though the object-mapping/build process (which happens on read) or by the user. In this way, this would provide a general mechanism to distinguish container behavior between reading from disk and creating new containers.

oruebel avatar May 19 '22 23:05 oruebel