EXP: construct proper np.memmap objects for memory-mapped numpy arrays in astropy.io.fits#19502
Draft
astrofrog wants to merge 3 commits intoastropy:mainfrom
Draft
EXP: construct proper np.memmap objects for memory-mapped numpy arrays in astropy.io.fits#19502astrofrog wants to merge 3 commits intoastropy:mainfrom
astrofrog wants to merge 3 commits intoastropy:mainfrom
Conversation
astrofrog
added
Experimental
needs-discussion
Extra CI
github-actions
bot
added
io.fits
utils
and removed
Extra CI
Contributor
|
Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.
|
astrofrog
added
the
Build all wheels
Member
Author
|
@pllim - I'm not sure I would milestone this quite yet, I doubt we will merge it 😅 |
Member
|
Re: milestone -- Just to get the check to pass. We will re-milestone anyway at branching time, right? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
NOTE: this is not ready for looking at yet, I need to experiment with it more!
This is an experimental PR inspired by discussion and comments from @saimn in #18665 - the question here is could we actually use proper
np.memmapobjects instead of usingnp.ndarraybacked by a mmap.The main thing this would break is if someone had something like
if type(hdu.data) is np.ndarray, but things likeisinstance(hdu.data, np.ndarray)would continue to work, and all array usage should just work as far as I can see.The benefit of doing this is that it makes it easier for users and downstream libraries to (e.g. reproject) to have access to the memory map parameters, which can then help decide on strategies for e.g. serializing in a multiprocessing environment, but there might also be other benefits. At the very least it makes it obvious to users if an array is memory mapped since otherwise there isn't actually a way to know from the array if it is memory mapped or in memory.
This is up for discussion/experimenting with, not for merging at this point - we might decide we don't want to do it.