Skip to content
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

MRG: expanding, rethinking mmap'ing #267

Merged
merged 9 commits into from
Nov 11, 2014
Merged

Conversation

matthew-brett
Copy link
Member

Expanding the tests for the bugfix suggested by @MichielCottaar

Trying to work out the previous logic broken for a long time by using Openers.

Check that BytesIO raises UnsupportedOperation, and other types have
non-zero fileno.
Test that ``array_from_file`` works correctly with ``Opener`` instances.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) when pulling fc08bc6 on matthew-brett:master into 5d085c6 on nipy:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) when pulling f2ce8f3 on matthew-brett:master into 5d085c6 on nipy:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) when pulling bbf6195 on matthew-brett:master into 5d085c6 on nipy:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) when pulling 6f1f188 on matthew-brett:master into 5d085c6 on nipy:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) when pulling a819a90 on matthew-brett:master into 5d085c6 on nipy:master.

@matthew-brett matthew-brett changed the title WIP: expanding, rethining mmap'ing MRG: expanding, rethinking mmap'ing Nov 7, 2014
@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) when pulling bb2eab4 on matthew-brett:master into 5d085c6 on nipy:master.

bz2 file does not have fileno in Python 3.2 and 2.7 on travis at least.
Test for gzip / bz2 file object, so we can in due course avoid
memory-mapping these guys in ``array_from_file``.
If we can change strings loaded from files without messing up the result
of a subsequent load, then we can use the string memory as modifiable
memory backing the array returned from `array_from_file`.
Check that reading an array, modifying the array, then rereading the
array, returns the same result for both reads.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) when pulling e3f3d66 on matthew-brett:master into 5d085c6 on nipy:master.

Use test for compressed files so we don't mmap them.  Numpy was trying
to mmap bz2 files.

Following suggestion from Nathaniel, use `readinto` method of most
file-like objects to read data into given memory location, so we can be
sure that memory location is not used by some other object.

If we can't use `readinto`, use heuristic to guess whether we can re-use
memory from file bytes read for array memory store.

Use bytearray to read data into, to avoid complications with numpy
array.data buffers and Python 3 / "non-native" datatypes.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) when pulling 15ceb6c on matthew-brett:master into 5d085c6 on nipy:master.

@matthew-brett
Copy link
Member Author

Benchmarks (python -c 'import nibabel; nibabel.bench()') show no signs of slowdown, so from my point of view this is ready to go once it has had a review.

This is the second to last PR before the release - we are in the home stretch - any takers?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.06%) when pulling d07a863 on matthew-brett:master into 5d085c6 on nipy:master.

Add tests that unscaled image returns memory map by default.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.07%) when pulling e2a3e77 on matthew-brett:master into 5d085c6 on nipy:master.

@@ -73,6 +76,14 @@ def test_Opener_various():
with Opener(input, 'rb') as fobj:
message_back = fobj.read()
assert_equal(message, message_back)
if input is sobj:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bit weird to have if input == sobj above and is sobj here, even though they probably have the same truth conditions here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks - good point changed.

Anyone have a look at this, the next-to-last PR before release?

@larsoner
Copy link
Contributor

larsoner commented Nov 9, 2014

This looks reasonable to me, but someone else more experienced with the code should probably take a look.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.07%) when pulling 35d0edd on matthew-brett:master into 5d085c6 on nipy:master.

@matthew-brett
Copy link
Member Author

Last call for review. I will merge later today unless I hear otherwise.

matthew-brett added a commit that referenced this pull request Nov 11, 2014
MRG: expanding, rethinking mmap'ing

Expanding the tests for the bugfix suggested by @MichielCottaar

Work out the previous logic for file mapping broken for a long time by using Openers.
@matthew-brett matthew-brett merged commit 5c15ed8 into nipy:master Nov 11, 2014
@matthew-brett matthew-brett deleted the master branch November 11, 2014 23:46
grlee77 pushed a commit to grlee77/nibabel that referenced this pull request Mar 15, 2016
MRG: expanding, rethinking mmap'ing

Expanding the tests for the bugfix suggested by @MichielCottaar

Work out the previous logic for file mapping broken for a long time by using Openers.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants