-
Notifications
You must be signed in to change notification settings - Fork 25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
replace usages of copy_arrays
with memmap
#306
replace usages of copy_arrays
with memmap
#306
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ 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 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Much simpler :) Good idea!
follow-on to asdf-format/asdf#1797
Checklist
CHANGES.rst
(either inBug Fixes
orChanges to API
)