spacetelescope / stdatamodels

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

remove extra asdf validation during DataModel.clone #230

Closed braingram closed 9 months ago

braingram commented 10 months ago

This PR changes what happens during DataModel.clone to avoid an extra validation of the asdf tree. The validation during clone is (without this PR) only performed if the deepcopy argument is True. clone with deepcopy=True is only called during copy where we can assume that the instanced being copied is valid. https://github.com/spacetelescope/stdatamodels/blob/ed1c5f6ee6d361b6a0b564af66b766b661cb704d/src/stdatamodels/model_base.py#L421-L429

Regression tests: https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/1021/ show 8 failures (the same as on jwst main).

While working an issue with cubebuild I noticed that 16% of the time spent during test_cube_build_single_output is validating asdf contents (in part during DataModel.clone). With this PR that number drops to 12% and the runtime of the test is sped up by ~10%.

Checklist

codecov[bot] commented 10 months ago

Codecov Report

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

Comparison is base (6884f83) 64.70% compared to head (f0adb6c) 64.70%.

:exclamation: Current head f0adb6c differs from pull request most recent head d0f1ca5. Consider uploading reports for the commit d0f1ca5 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #230 +/- ## ======================================= Coverage 64.70% 64.70% ======================================= Files 102 102 Lines 5667 5667 ======================================= Hits 3667 3667 Misses 2000 2000 ```

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

hbushouse commented 9 months ago

The 8 regression tests failures are unrelated and caused by some new CRDS ref files back at the time the tests were run.