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

saving image to a "new" file, which is a symlink... what should happen? #465

Open
yarikoptic opened this issue Jul 29, 2016 · 6 comments
Open

Comments

@yarikoptic
Copy link
Member

yarikoptic commented Jul 29, 2016

ATM

  • even if intend to write to a "new" file, if file exists and a symlink, it would write into a file pointed by the symlink... (well -- to say the truth -- this behavior is consistent with e.g. how > in shell works):
In [1]: import nibabel as nib

In [2]: !ln -s nonexistent out.nii.gz

In [3]: !ls -l nonexistent out.nii.gz
ls: cannot access 'nonexistent': No such file or directory
lrwxrwxrwx 1 yoh yoh 11 Jul 28 23:29 out.nii.gz -> nonexistent

*In [7]: nib.Nifti1Image(np.arange(12).reshape((2,2,3)), None, None).to_filename('out.nii.gz')

In [8]: !ls -l nonexistent out.nii.gz
-rw------- 1 yoh yoh 96 Jul 28 23:30 nonexistent
lrwxrwxrwx 1 yoh yoh 11 Jul 28 23:29 out.nii.gz -> nonexistent
  • if file pointed by a symlink exists but with permissions removed for writing (belongs to other user, or explicitly write bit removed, e.g. under annex):

In [9]: !chmod a-w nonexistent

In [10]: nib.Nifti1Image(np.arange(12).reshape((2,2,3)), None, None).to_filename('out.nii.gz')
...
/home/yoh/proj/nipy/nipy-suite/nibabel/nibabel/openers.py in _gzip_open(fileish, *args, **kwargs)
     62 
     63 def _gzip_open(fileish, *args, **kwargs):
---> 64     gzip_file = BufferedGzipFile(fileish, *args, **kwargs)
     65 
     66     # Speedup for #209; attribute not present in in Python 3.5

/usr/lib/python2.7/gzip.pyc in __init__(self, filename, mode, compresslevel, fileobj, mtime)
     92             mode += 'b'
     93         if fileobj is None:
---> 94             fileobj = self.myfileobj = __builtin__.open(filename, mode or 'rb')
     95         if filename is None:
     96             # Issue #13781: os.fdopen() creates a fileobj with a bogus name

IOError: [Errno 13] Permission denied: 'out.nii.gz'

I wondered... would it be sensible to suggest to first remove the file intended to be written to (at least in to_filename) before writing? that would allow to interact with git-annex'ed datasets more easily,

@matthew-brett
Copy link
Member

Just to recap for myself - at the moment, if you ask nibabel to write to a symlink, it will write to the file pointed to by the symlink. Here you're proposing deleting the symlink and replacing it with the written file.

This seems like it could get very confusing for people upgrading nibabel to find that their files get written to a different location.

Any thoughts from the rest of the team?

@yarikoptic
Copy link
Member Author

Yeah... As I have noted in the PR I agree that it is quite suboptimal in that regard. Would you mind if i suggest git-annex specific handling? Ie if it is a symlinks and has .git/annex/objects in its resolved path?

On July 29, 2016 9:30:22 PM EDT, Matthew Brett [email protected] wrote:

Just to recap for myself - at the moment, if you ask nibabel to write
to a symlink, it will write to the file pointed to by the symlink.
Here you're proposing deleting the symlink and replacing it with the
written file.

This seems like it could get very confusing for people upgrading
nibabel to find that their files get written to a different location.

Any thoughts from the rest of the team?


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
#465 (comment)

@matthew-brett
Copy link
Member

Ouch - feels like a hack. Well - because it is a hack. I guess you can't get away with a custom 'save' function that does the deleting for you, because you want all nibabel processing to work transparently with git annex?

@yarikoptic
Copy link
Member Author

Yeah

On July 29, 2016 11:06:29 PM EDT, Matthew Brett [email protected] wrote:

Ouch - feels like a hack. Well - because it is a hack. I guess you
can't get away with a custom 'save' function that does the deleting for
you, because you want all nibabel processing to work transparently with
git annex?


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
#465 (comment)

@effigies
Copy link
Member

So for existing files, you have three options: lone file, hard link, symlink. Is there any context in which one would want (or expect) to truncate a file and not hard links to it? If not, we can at least reduce the scope to symlinks.

Even then, I would say that deleting symlinks is not the expected default behavior, so maybe this could add a delete_symlinks=False as a default parameter for to_filename? Which would also need to exist in nibabel.save.

Or, taking the environment variable route, you could set NIBABEL_REMOVE_BEFORE_WRITE=1 and set the default parameter to delete_symlinks=os.environ.get('NIBABEL_REMOVE_BEFORE_WRITE', False).

@yarikoptic
Copy link
Member Author

FWIW, updated #466 with annex specific hack and env variable handling. If we add a kwarg there, I would make it delete_symlinks=None and then make it adhere to suggested annex/envvar logic iff None, and otherwise strictly following bool value (False -- never removes, True - always removes)... makes sense?

@effigies effigies modified the milestone: 2.5.0 Apr 25, 2019
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

3 participants