spacetelescope / stdatamodels

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

allow control of memmapping for asdf files #232

Closed braingram closed 9 months ago

braingram commented 10 months ago

This PR allows control of memory mapping when opening an asdf file using DataModel(asdf_filename). Control of memory mapping already exists for FITS files (through the memmap argument which is reused here).

asdf is making a few changes to memory mapping. This PR will help to prepare stdatamodels for those changes. Specifically, asdf will no longer force closure of memory mapped view of arrays (which can and often results in a segfault when an array access fails during an attempt to access the closed file). Instead, asdf will release all references to the opened memory map and rely on the garbage collector to close the file (see this asdf PR for more details: https://github.com/asdf-format/asdf/pull/1668). This works in most situations but is complicated in a few stdatamodels tests which test for the number of open files without first destroying the model. This PR (by using the default memmap=False) allows the effected tests to pass as they no longer use memory mapping.

Regression tests: https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/1025/ show only existing 8 unrelated failures (due to niriss crds file updates).

Checklist

codecov[bot] commented 9 months ago

Codecov Report

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

Comparison is base (1180473) 64.77% compared to head (8515e2b) 64.78%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #232 +/- ## ======================================= Coverage 64.77% 64.78% ======================================= Files 103 103 Lines 5687 5688 +1 ======================================= + Hits 3684 3685 +1 Misses 2003 2003 ```

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