-
Notifications
You must be signed in to change notification settings - Fork 26
Remove memmap option #507
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
Remove memmap option #507
Conversation
|
JWST regression tests started here https://github.com/spacetelescope/RegressionTests/actions/runs/15881902672 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #507 +/- ##
==========================================
+ Coverage 81.02% 81.07% +0.04%
==========================================
Files 110 110
Lines 5422 5431 +9
==========================================
+ Hits 4393 4403 +10
+ Misses 1029 1028 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@drlaw1558 How do these deprecation messages look to you? |
|
LG2M |
|
This PR needs to wait until spacetelescope/jwst#9593 is merged; otherwise the warnings are emitted whenever a ModelContainer is initialized |
|
spacetelescope/jwst#9593 is merged, rerunning regtests here: https://github.com/spacetelescope/RegressionTests/actions/runs/16123598864 |
|
Looks good, pending clean regtests. Did this one have an associated JP ticket? I know we asked INS if it was okay to remove the argument, did we hear a final word on that? |
|
It did not have an associated ticket. David did bring it up in a JP coordination meeting and then subsequently gave me the go-ahead after hearing that nobody knew memmap was even an option - at least, that's how I interpreted it. |
|
regtests are clean |
melanieclarke
left a comment
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.
LGTM, but I'll hold off on approval to see if @braingram wants to review/comment.
|
Let's make the next version of stdatamodels 4.0.0 since this is a breaking change. |
Closes #506
This PR hard-codes
memmaptoFalsewhen reading FITS and ASDF, and emits a warning when a user attempts to use memory mapping.Tasks
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