spacetelescope / stdatamodels

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

Retain `id` and other top-level attributes in `merge_property_trees` #364

Closed braingram closed 4 days ago

braingram commented 6 days ago

This PR updates merge_property_trees to retain the schema id (and other "top level" items/attributes). This fixes the issue where model.schema["id"] does not match the "id" of the schema (see #308).

This PR also drops the use of OrderedDict fixing #40.

Finally the CI goblins revealed that shuffling order of tests can cause test_defined_models_up_to_date to fail: https://github.com/spacetelescope/stdatamodels/actions/runs/11901768169/job/33165405951 if executed after test_model_with_nonstandard_primary_array which defines a test model class that isn't prefixed with _. This PR adds a _ prefix to the test class to avoid this intermittent issue.

Fixes: https://jira.stsci.edu/browse/JP-2137 Closes #308 Closes #40

Regression tests: https://github.com/spacetelescope/RegressionTests/actions/runs/11914331392 produce 5 "failures" which look like improvements to me. The failures are in the niriss ami tests:

        Headers contain differences:
         Keyword          has different values:
            a> AMI OIFITS analysis data model
            b> OIFITS required meta data (not otherwise defined in core.schema)

Since this PR retains the top level schema items this is keeping the title from the datamodel schema: https://github.com/spacetelescope/stdatamodels/blob/2c3985336e54fb4989460b08a94777dfb33640b7/src/stdatamodels/jwst/datamodels/schemas/amioi.schema.yaml#L5 which in the truth file ends up being overwritten by the referenced oifits schema: https://github.com/spacetelescope/stdatamodels/blob/2c3985336e54fb4989460b08a94777dfb33640b7/src/stdatamodels/jwst/datamodels/schemas/oifits.schema.yaml#L5 Keeping the datamodel schema title seems preferable.

Tasks

news fragment change types... - ``changes/.feature.rst``: new feature - ``changes/.bugfix.rst``: fixes an issue - ``changes/.doc.rst``: documentation change - ``changes/.removal.rst``: deprecation or removal of public API - ``changes/.misc.rst``: infrastructure or miscellaneous change
codecov[bot] commented 6 days ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 67.56%. Comparing base (1e16207) to head (3aad51e). Report is 5 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #364 +/- ## ========================================== + Coverage 67.52% 67.56% +0.03% ========================================== Files 114 114 Lines 5916 5919 +3 ========================================== + Hits 3995 3999 +4 + Misses 1921 1920 -1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.


🚨 Try these New Features:

braingram commented 4 days ago

I'm going to wait to merge and okify this until tomorrow.

braingram commented 4 days ago

Regtest started for okifying: https://github.com/spacetelescope/RegressionTests/actions/runs/11954122369