ccpem/mrcfile

Origin in header is read-only

alisterburt opened this issue ยท 9 comments

Hey @colinpalmer !

I'm trying to write a file with a modified origin and am running into

  File "", line 42, in main
    mrc.header.origin = origin_ang
  File "", line 506, in __setattr__
    return self.setfield(val, *res)
ValueError: assignment destination is read-only

I'm getting the same when I try to assign to the x, y or z fields individually. Is this intentional? Any chance we can make this writable?

Many thanks!

Alister

Hi Alister!

I've just tried to reproduce this one myself and I can't, everything seems to work normally for me. I can set the origin as a single field (as a tuple or record array) and I can assign to the x, y, z fields within it.

Just to check the obvious, are you sure that you've got the file open in writable mode? Can you edit the data and assign to any other header fields?

And in case it's relevant, what Python, numpy, mrcfile and OS versions are you using?

Edited to add: I've pushed up a commit that adds some test cases for this. It'd be great if you could check whether or not they pass on your system.

Uh oh, I made the classic error of reporting a bug for user error! ๐Ÿ˜†

I was doing

with mrcfile.open('file.mrc', header_only=True) as mrc:
    bla

Without setting the mode to writable - for some reason I only thought about the mode as a property of the data, not the header.

My bad, thanks a bunch for getting back so quickly!

actually, there is something that is still not quite right here:

this simple test

import mrcfile
import numpy as np

a = np.random.normal(size=(28, 28, 28))
mrcfile.write('test.mrc', a)

with open('test.mrc', header_only=True, mode='w') as mrc:
    mrc.header.origin = (7, 8, 9)

fails with

Traceback (most recent call last):
  File "/Users/burta2/test_data/test.py", line 7, in <module>
    with mrcfile.open('test.mrc', header_only=True, mode='w+') as mrc:
  File "/Users/burta2/mambaforge/envs//lib/python3.10/site-packages/mrcfile/load_functions.py", line 139, in open
    return NewMrc(name, mode=mode, permissive=permissive,
  File "/Users/burta2/mambaforge/envs/lib/python3.10/site-packages/mrcfile/mrcfile.py", line 103, in __init__
    raise ValueError("File '{0}' already exists; set overwrite=True "
ValueError: File 'test.mrc' already exists; set overwrite=True to overwrite it

There is no way to pass overwrite to the MrcFile constructor from mrcfile.open so a user would currently have to manually construct a MrcFile instance to modify this.

return NewMrc(name, mode=mode, permissive=permissive,
header_only=header_only)

could be replaced by

    overwrite = True if 'w' in mode else False
    return NewMrc(name, mode=mode, permissive=permissive,
                  header_only=header_only, overwrite=overwrite)

but I'm not sure this is 100% correct and defer to you :-)

it's definitely not correct, doing what I suggested leads to corrupted outputs ๐Ÿ˜†

it's definitely not correct, doing what I suggested leads to corrupted outputs ๐Ÿ˜†

Good to know!

I don't think I'll make any changes to the logic here, anyway. I definitely wouldn't set overwrite=True just from the mode setting - that would defeat the entire point of having the overwrite flag in the first place! IMO overwriting an existing file should always be an explicit decision by the programmer. Usually if someone's making a new file they'd use mrcfile.new (which has an overwrite option) rather than mrcfile.open(mode='w'), and if they really want to use mrcfile.open() for some reason I'd suggest the best approach would be to use os.remove() (or similar) to delete the existing file first, which makes the intent to replace the file very clear.

The only issue I can see in the example you gave is that the message that says "set overwrite=True to overwrite it" gets passed unchanged from the MrcFile constructor through the mrcfile.open() function at which point that part of the message is no longer valid. I'll try to correct the message in that case so it's a bit more helpful.

I've improved the error message for mrcfile.open. Closing this issue for now, but do reopen it if there's still a problem.

Wait, sorry - what I'm explicitly trying to do in this particular case is modify the header of an existing file, it feels unintuitive that this isn't possible in 'w' modes. Are my expectations off here?

Edit: sorry, this exact use case definitely wasn't clear from the beginning

Just like the Python open() function, mode='w' means "write a new file". To edit an existing file you want mode='r+'

Gotcha, ta!