spacetelescope / stdatamodels

https://stdatamodels.readthedocs.io
Other
5 stars 25 forks source link

datamodels doesn't validate arrays in some cases #307

Open jdavies-st opened 5 years ago

jdavies-st commented 5 years ago

jwst.datamodels validates array sizes on reading from a FITS file or on assignment. So ImageModel requires the data array to be 2D, and so if I try to read or assign a 3D array, it fails validation:

In [22]: m = datamodels.ImageModel()

In [24]: m.data = np.arange(10*10).reshape((1, 10, 10))
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-24-d91d60f9cc1c> in <module>
----> 1 m.data = np.arange(10*10).reshape((1, 10, 10))

~/dev/jwst/jwst/datamodels/model_base.py in __setattr__(self, attr, value)
    623             object.__setattr__(self, attr, value)
    624         elif ndmodel.NDModel.my_attribute(self, attr):
--> 625             ndmodel.NDModel.__setattr__(self, attr, value)
    626         else:
    627             properties.ObjectNode.__setattr__(self, attr, value)

~/dev/jwst/jwst/datamodels/ndmodel.py in data(self, value)
    130         if not primary_array_name:
    131             primary_array_name = 'data'
--> 132         properties.ObjectNode.__setattr__(self, primary_array_name, value)
    133 
    134     @property

~/dev/jwst/jwst/datamodels/properties.py in __setattr__(self, attr, val)
    258             if val is None:
    259                 val = _make_default(attr, schema, self._ctx)
--> 260             val = _cast(val, schema)
    261 
    262             node = ObjectNode(attr, val, schema, self._ctx)

~/dev/jwst/jwst/datamodels/properties.py in _cast(val, schema)
     37             raise ValueError(
     38                 "Array has wrong number of dimensions.  Expected {0}, got {1}".format(
---> 39                     schema['ndim'], len(val.shape)))
     40 
     41         if 'max_ndim' in schema and len(val.shape) > schema['max_ndim']:

ValueError: Array has wrong number of dimensions.  Expected 2, got 3

That's good.

But if I instantiate an ImageModel with a 3D array or from an already existing datamodel with a 3D array, it doesn't do any validation and allows it.

In [25]: m = datamodels.ImageModel((1, 10, 10))

In [26]: cube = datamodels.CubeModel((1, 10, 10))

In [27]: m = datamodels.ImageModel(cube)

This is incorrect. It should be giving the same validation error as above.

philhodge commented 5 years ago

I understand (well, maybe) why one would want different models for 2-D and 3-D arrays, but your example brought to mind a possible snag. We often reshape an array to add an axis of length 1, for the purpose of broadcasting with another array. Explicitly specifying where the axis of length 1 falls within the shape may be necessary if it isn't the last axis; the irs2 code in refpix (translated from IDL) does this a lot. If an array was reshaped without first making a copy, and a validation check was done while a (10, 10) array temporarily had shape (1, 10, 10), that would not be helpful.

jdavies-st commented 5 years ago

I think as long as you're not assigning it, it should be fine:

In [3]: m = datamodels.ImageModel()

In [4]: m.data = np.arange(10*10).reshape((10, 10))

In [5]: m.data
Out[5]: 
array([[ 0.,  1.,  2.,  3.,  4.,  5.,  6.,  7.,  8.,  9.],
       [10., 11., 12., 13., 14., 15., 16., 17., 18., 19.],
       [20., 21., 22., 23., 24., 25., 26., 27., 28., 29.],
       [30., 31., 32., 33., 34., 35., 36., 37., 38., 39.],
       [40., 41., 42., 43., 44., 45., 46., 47., 48., 49.],
       [50., 51., 52., 53., 54., 55., 56., 57., 58., 59.],
       [60., 61., 62., 63., 64., 65., 66., 67., 68., 69.],
       [70., 71., 72., 73., 74., 75., 76., 77., 78., 79.],
       [80., 81., 82., 83., 84., 85., 86., 87., 88., 89.],
       [90., 91., 92., 93., 94., 95., 96., 97., 98., 99.]], dtype=float32)

In [6]: m.data.reshape((1, 10, 10))
Out[6]: 
array([[[ 0.,  1.,  2.,  3.,  4.,  5.,  6.,  7.,  8.,  9.],
        [10., 11., 12., 13., 14., 15., 16., 17., 18., 19.],
        [20., 21., 22., 23., 24., 25., 26., 27., 28., 29.],
        [30., 31., 32., 33., 34., 35., 36., 37., 38., 39.],
        [40., 41., 42., 43., 44., 45., 46., 47., 48., 49.],
        [50., 51., 52., 53., 54., 55., 56., 57., 58., 59.],
        [60., 61., 62., 63., 64., 65., 66., 67., 68., 69.],
        [70., 71., 72., 73., 74., 75., 76., 77., 78., 79.],
        [80., 81., 82., 83., 84., 85., 86., 87., 88., 89.],
        [90., 91., 92., 93., 94., 95., 96., 97., 98., 99.]]],
      dtype=float32)

In [7]: m.data.reshape((1, 10, 10)).shape
Out[7]: (1, 10, 10)