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

The nrrd.read() function returns a read-only numpy array if the input file is compressed #99

Closed
lguyot opened this issue Oct 11, 2019 · 5 comments

Comments

@lguyot
Copy link

lguyot commented Oct 11, 2019

Imagine that you need to load into memory a very large image file which has for instance a gzip encoding.

Using the following snippet

import nrrd
img, options = nrrd.read('very_large_image_file.nrrd') # compressed with gzip encoding
print(img.flags)

you would get the following output

 C_CONTIGUOUS : False
  F_CONTIGUOUS : True
  OWNDATA : False
  WRITEABLE : False
  ALIGNED : True
  WRITEBACKIFCOPY : False
  UPDATEIFCOPY : False

This means that the numpy array named img is read-only and hence a deep copy is required for any write operation on this array. Hence we need to double the allocated memory every time we want to write on a numpy array loaded from a compressed nrrd file.

If the file is not compressed, e.g., its encoding is raw, the output is:

  C_CONTIGUOUS : True
  F_CONTIGUOUS : True
  OWNDATA : True
  WRITEABLE : True
  ALIGNED : True
  WRITEBACKIFCOPY : False
  UPDATEIFCOPY : False

In this case, the returned numpy array is writeable, which is not consistent with the previous behavior.

The issue may come from line 446 in reader.py:

data = np.frombuffer(decompressed_data[byte_skip:], dtype)

If data is generated from a non-writeable buffer, np.frombuffer returns a read-only numpy array.

@addisonElliott
Copy link
Collaborator

Hello,

Thanks for the issue. I fixed something similar to this by switching from np.frombuffer to np.fromstring. You can see the PR/commit here: 6b158c6

It will be a bit before I have some time to work on this myself, as always, PRs are welcome. Follow the same setup as my PR, add some unit tests that demonstrate the problem is fixed.

@lguyot
Copy link
Author

lguyot commented Oct 11, 2019

Hello,

Many thanks for your prompt reply. You actually fixed the issue with 6b158c6 but you reverted this change when improving the performances of nrrd.read() in 16be776.

If np.fromstring() is not dramatically slower than np.frombuffer(), we would only need to reinstate this single line, namely 446, of reader.py as it was in 6b158c6.

@addisonElliott
Copy link
Collaborator

Hm, well the odd thing is that the unit tests should have failed after reverting that change.

@lguyot
Copy link
Author

lguyot commented Oct 11, 2019

Indeed, they are all passing. I assumed mistakenly that I was using the latest version of pynrrd.
But this is not the case: I am using pynrrd==0.2.5 where the issue can be reproduced.

This is not the case with version 0.4.0.

You can safely close this issue. I apologize for the trouble.

@lguyot lguyot closed this as completed Oct 14, 2019
@lguyot
Copy link
Author

lguyot commented Oct 14, 2019

The problem has been definitively fixed since version 0.3.4. My report is threfore not relevent. No action need to be taken.

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

No branches or pull requests

2 participants