From 6662db8f721cddedaf9ae681d33c6be98b7c1e2b Mon Sep 17 00:00:00 2001 From: Ned Molter Date: Mon, 10 Feb 2025 11:24:55 -0500 Subject: [PATCH 1/4] Fix lack of validation of dimension when instantiating datamodel from shape --- src/stdatamodels/properties.py | 4 ++++ tests/test_models.py | 3 +++ 2 files changed, 7 insertions(+) diff --git a/src/stdatamodels/properties.py b/src/stdatamodels/properties.py index dcd8abbb..bedd5247 100644 --- a/src/stdatamodels/properties.py +++ b/src/stdatamodels/properties.py @@ -180,6 +180,10 @@ def _make_default_array(attr, schema, ctx): if attr == primary_array_name: if ctx.shape is not None: + # Ensure that input shape matches schema ndim + if (ndim is not None) and (len(ctx.shape) != ndim): + msg = f"Array has wrong number of dimensions. Expected {ndim}, got {len(ctx.shape)}" + raise ValueError(msg) shape = ctx.shape elif ndim is not None: shape = tuple([0] * ndim) diff --git a/tests/test_models.py b/tests/test_models.py index e76e0e5a..63db8c1b 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -99,6 +99,9 @@ def test_init_with_array2(): with BasicModel(array) as dm: dm.data # noqa: B018 +def test_init_invalid_shape(): + with pytest.raises(ValueError): + BasicModel((50, 50, 50)) def test_set_array(): with pytest.raises(ValueError): From 291c678914f81978198c00f77c281d78f8a87695 Mon Sep 17 00:00:00 2001 From: Ned Molter Date: Mon, 10 Feb 2025 11:50:08 -0500 Subject: [PATCH 2/4] Fix unit test, add changelog --- changes/395.bugfix.rst | 1 + src/stdatamodels/jwst/datamodels/tests/test_models.py | 2 +- tests/test_models.py | 2 ++ 3 files changed, 4 insertions(+), 1 deletion(-) create mode 100644 changes/395.bugfix.rst diff --git a/changes/395.bugfix.rst b/changes/395.bugfix.rst new file mode 100644 index 00000000..23b0f072 --- /dev/null +++ b/changes/395.bugfix.rst @@ -0,0 +1 @@ +Validate dimension against schema when instantiating datamodel from array shape diff --git a/src/stdatamodels/jwst/datamodels/tests/test_models.py b/src/stdatamodels/jwst/datamodels/tests/test_models.py index bfc1dd59..cd3fefde 100644 --- a/src/stdatamodels/jwst/datamodels/tests/test_models.py +++ b/src/stdatamodels/jwst/datamodels/tests/test_models.py @@ -142,7 +142,7 @@ class _NonstandardPrimaryArrayModel(JwstDataModel): def get_primary_array_name(self): return "wavelength" - m = _NonstandardPrimaryArrayModel((10,)) + m = _NonstandardPrimaryArrayModel((10, 10)) assert "wavelength" in list(m.keys()) assert m.wavelength.sum() == 0 diff --git a/tests/test_models.py b/tests/test_models.py index 63db8c1b..146cb903 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -99,10 +99,12 @@ def test_init_with_array2(): with BasicModel(array) as dm: dm.data # noqa: B018 + def test_init_invalid_shape(): with pytest.raises(ValueError): BasicModel((50, 50, 50)) + def test_set_array(): with pytest.raises(ValueError): with BasicModel() as dm: From dc8169089e7158c680cc3dcf4ed21b638f3b0ab9 Mon Sep 17 00:00:00 2001 From: Ned Molter Date: Mon, 10 Feb 2025 20:07:12 -0500 Subject: [PATCH 3/4] Added handling for max_ndim --- src/stdatamodels/properties.py | 33 +++++++++++++++++++++++++++++---- tests/test_models.py | 13 ++++++++++++- 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/src/stdatamodels/properties.py b/src/stdatamodels/properties.py index bedd5247..a88b5987 100644 --- a/src/stdatamodels/properties.py +++ b/src/stdatamodels/properties.py @@ -180,11 +180,8 @@ def _make_default_array(attr, schema, ctx): if attr == primary_array_name: if ctx.shape is not None: - # Ensure that input shape matches schema ndim - if (ndim is not None) and (len(ctx.shape) != ndim): - msg = f"Array has wrong number of dimensions. Expected {ndim}, got {len(ctx.shape)}" - raise ValueError(msg) shape = ctx.shape + _validate_primary_shape(schema, shape) elif ndim is not None: shape = tuple([0] * ndim) else: @@ -226,6 +223,34 @@ def _make_default_array(attr, schema, ctx): return array +def _validate_primary_shape(schema, shape): + """ + Ensure requested shape is allowed by schema. + + Parameters + ---------- + schema : dict + The schema for the primary array. + shape : tuple + The requested shape of the default array. + + Raises + ------ + ValueError + If the requested has dimensions different from the schema's ndim, + or larger than the schema's max_ndim. + """ + ndim_requested = len(shape) + max_ndim = schema.get("max_ndim", None) + ndim = schema.get("ndim", None) + if (ndim is not None) and (ndim_requested != ndim): + msg = f"Array has wrong number of dimensions. Expected {ndim}, got {ndim_requested}" + raise ValueError(msg) + if (max_ndim is not None) and (ndim_requested > max_ndim): + msg = f"Array has wrong number of dimensions. Expected <= {max_ndim}, got {ndim_requested}" + raise ValueError(msg) + + def _make_default(attr, schema, ctx): if "max_ndim" in schema or "ndim" in schema or "datatype" in schema: return _make_default_array(attr, schema, ctx) diff --git a/tests/test_models.py b/tests/test_models.py index 146cb903..5b7ae4c6 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -101,8 +101,19 @@ def test_init_with_array2(): def test_init_invalid_shape(): + """Requested some number of dimensions unequal to ndim""" with pytest.raises(ValueError): - BasicModel((50, 50, 50)) + BasicModel((50,)) + + +def test_init_invalid_shape2(): + """Requested more dimensions than max_ndim""" + with BasicModel() as dm: + schema = dm._schema + schema["properties"]["data"]["max_ndim"] = 1 + schema["properties"]["data"]["ndim"] = None + with pytest.raises(ValueError): + BasicModel((50, 50), schema=schema) def test_set_array(): From 13e138f98954857dd32ded629ccc3fb61289c6d6 Mon Sep 17 00:00:00 2001 From: Ned Molter Date: Tue, 11 Feb 2025 13:25:52 -0500 Subject: [PATCH 4/4] small fix from code review --- tests/test_models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_models.py b/tests/test_models.py index 5b7ae4c6..0b49b246 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -101,7 +101,7 @@ def test_init_with_array2(): def test_init_invalid_shape(): - """Requested some number of dimensions unequal to ndim""" + """Requested some number of dimensions unequal to ndim, which is set to 2""" with pytest.raises(ValueError): BasicModel((50,))