Validate dimension when instantiating datamodel from shape #395
Conversation
|
regression tests started here https://github.com/spacetelescope/RegressionTests/actions/runs/13245788321 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #395 +/- ##
==========================================
+ Coverage 78.16% 78.21% +0.04%
==========================================
Files 115 115
Lines 5144 5155 +11
==========================================
+ Hits 4021 4032 +11
Misses 1123 1123 ☔ View full report in Codecov by Sentry. |
| @@ -142,7 +142,7 @@ class _NonstandardPrimaryArrayModel(JwstDataModel): | |||
| def get_primary_array_name(self): | |||
| return "wavelength" | |||
|
|
|||
| m = _NonstandardPrimaryArrayModel((10,)) | |||
| m = _NonstandardPrimaryArrayModel((10, 10)) | |||
There was a problem hiding this comment.
This appeared to be an instance of an array passing validation that we would expect to fail, and indeed it does fail as of the bugfix in this PR. It doesn't look like this change makes much difference for what the test intends to cover, but someone can correct me if that's wrong.
|
https://github.com/spacetelescope/RegressionTests/actions/runs/13245788321 are all passing now |
|
@jdavies-st Not sure why I can't request you as a reviewer, so pinging you in the comments to take a look |
tests/test_models.py
Outdated
| @@ -100,6 +100,22 @@ def test_init_with_array2(): | |||
| dm.data # noqa: B018 | |||
|
|
|||
|
|
|||
| def test_init_invalid_shape(): | |||
| """Requested some number of dimensions unequal to ndim""" | |||
There was a problem hiding this comment.
You may want just mention in the docstring here what the expected dimension of BasicModel.data should be.
|
@jdavies-st I already ran the jwst regtests here: https://github.com/spacetelescope/RegressionTests/actions/runs/13245788321 Romancal no longer uses stdatamodels. |
It seems this repo only accepts reviews from people with write access to the repo. I'm no longer internal at STScI, so I don't have write access here. So my review above looks like a normal comment, not a review. |
Gotcha, well thanks for looking anyway. I can see your approval, but it shows up as a gray checkmark instead of a green checkmark |
Closes #307
This PR addresses a bug when instantiating datamodels from an array shape. The problem is discussed in detail on the issue, but in short, the input shape was not checked against the schema
ndimattribute when creating a default arrayTasks
docs/pageno-changelog-entry-needed)changes/:echo "changed something" > changes/<PR#>.<changetype>.rst(see below for change types)jwstregression tests with this branch installed ("git+https://github.com/<fork>/stdatamodels@<branch>")news fragment change types...
changes/<PR#>.feature.rst: new featurechanges/<PR#>.bugfix.rst: fixes an issuechanges/<PR#>.doc.rst: documentation changechanges/<PR#>.removal.rst: deprecation or removal of public APIchanges/<PR#>.misc.rst: infrastructure or miscellaneous change