search

spacetelescope / stdatamodels

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

replace usages of ``copy_arrays`` with ``memmap`` #306

Closed zacharyburnett closed 3 months ago

zacharyburnett commented 3 months ago

follow-on to https://github.com/asdf-format/asdf/pull/1797

Checklist

codecov[bot] commented 3 months ago

Codecov Report

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

Project coverage is 66.21%. Comparing base (4d7c3a6) to head (f5c46d7). Report is 92 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #306 +/- ## ========================================== + Coverage 64.84% 66.21% +1.37% ========================================== Files 103 101 -2 Lines 5694 5402 -292 ========================================== - Hits 3692 3577 -115 + Misses 2002 1825 -177 ```

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

zacharyburnett commented 3 months ago

Thanks!

Based on https://github.com/spacetelescope/jwst/pull/8660/files the "copy_arrays" usage is very limited in jwst and I see no uses that pass it to DataModel.__init__. It's possible a user might be doing this and it also makes sense to me to remove "copy_arrays" as an allowed keyword argument (eventually).

Would you add a deprecation for passing "copy_arrays" to DataModel.__init__ and a test for the deprecation? I think another UserWarning is called for here since it seems likely to happen in an interactive session. We can then remove "copy_arrays" for stdatamodels 3.0.0

Alternatively, we can remove handling copy_arrays entirely and outsource that warning to the AsdfWarning that'll come up when it is passed to asdf.open(); see f5c46d761c3ea876bcd60843e3811a7d4b744a56