-
Notifications
You must be signed in to change notification settings - Fork 259
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
ENH: Add copy() method to ArrayProxy #1255
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1255 +/- ##
=======================================
Coverage 92.24% 92.24%
=======================================
Files 99 99
Lines 12452 12460 +8
Branches 2560 2561 +1
=======================================
+ Hits 11486 11494 +8
Misses 643 643
Partials 323 323 ☔ View full report in Codecov by Sentry. |
f0fcd8f
to
81b1033
Compare
@pauldmccarthy This is bound to interact with indexed gzip. If you have time for a quick read-through, I would appreciate your thoughts. |
81b1033
to
945a65a
Compare
Hi @effigies, apologies for the delay - I missed this mention. On the surface, I think this looks fine. In the typical case where an In the less common case where an If you like, you could add another (very rudimentary) test with something like this: def test_copy_with_indexed_gzip_handle():
import nibabel as nib
indexed_gzip = pytest.importorskip('indexed_gzip')
with InTemporaryDirectory():
# ~24MB
spec = ((50, 50, 50, 50), np.float32, 352, 1, 0)
data = np.arange(np.prod(spec[0]), dtype=np.float32).reshape(spec[0])
nib.Nifti1Image(data, np.eye(4)).to_filename('image.nii.gz')
with indexed_gzip.IndexedGzipFile('image.nii.gz') as gzf:
ap1 = nib.arrayproxy.ArrayProxy(gzf, spec)
ap2 = ap1.copy()
assert ap1.file_like is gzf
assert ap2.file_like is gzf
assert (ap1[ 0, 0, 0, :] == ap2[ 0, 0, 0, :]).all()
assert (ap1[-1, -1, -1, :] == ap2[-1, -1, -1, :]).all() |
945a65a
to
1c1845f
Compare
Thanks for the review and test! If you think of a more strenuous test or something we can do upstream, happy to discuss more. |
Test failure unrelated. |
Small addition to ArrayProxy class to allow copying. Used in #1090, but not central to it.