MRG: mmap keyword for loading images #268
Conversation
Add memory mapping keyword to array_from_file getting ready to add this to the image loading API.
Give array proxy class ability to tune memory mapping via `mmap` keyword-only argument.
Allow specifying mmap flags when loading Analyze-type images.
Check this works for mmap keyword argument to top-level load.
I want to use the arrayproxy tests for other ArrayProxy objects, particularly the PARRECArrayProxy. Refactor to make this easier.
Refactor image arrayproxy test mixin to make it easier to deal with the case where we can't easily make a new image from scratch, and we might want to use some example image instead.
Use refactored tests to test PARREC mmap interface.
I copy / pasted docstrings leaving the original argument name `infile`, for methods / functions where this is not the argument name. Fix.
matthew-brett
force-pushed
the
mmap-api
branch
from
7dad228 to
744c631
Compare
matthew-brett
changed the title
|
Pretty please for a review? We should be ready for release after this one is in. |
| `mmap` controls the use of numpy memory mapping for reading image | ||
| array data. If False, do not try numpy ``memmap`` for data array. | ||
| If one of {'c', 'r'}, try numpy memmap with ``mode=mmap``. A `mmap` | ||
| value of True gives the same behavior as ``mmap='c'``. If image |
There was a problem hiding this comment.
Why even allow mmap=True if it just wraps to mmap='c'? Can't you just say the default is 'c'?
There was a problem hiding this comment.
This was a discussion we had on the mailing list. The idea was that someone might want to turn on memory mapping, but not know about 'mode', so True is there as a convenience for that person. I don't feel crazy strongly about it though.
There was a problem hiding this comment.
Ahh, I suppose that makes sense. Makes sense to leave it that way, then.
|
I didn't follow the discussion on the list too closely, but from what I did understand this LGTM other than my minor gripes. |
Use 'A is not None' instead of 'not A is None'. Fix incorrect docstring mentions of 'r+'.
|
Did I get everything with the new commit? |
|
Yep, LGTM |
|
Thanks for the review, it was very useful. |
MRG: mmap keyword for loading images Add mmap keyword to control memory mapping on image load. API as discussed on the nipy list.
MRG: mmap keyword for loading images Add mmap keyword to control memory mapping on image load. API as discussed on the nipy list.
Add mmap keyword to control memory mapping on image load.
API as discussed on the nipy list.