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

Avoid overwriting by moving old file to backup #150

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ahmed-shariff
Copy link
Contributor

This PR ensures the FileSaver doesn't overwrite any existing files. It creates a backup of a file about to be overwritten with it's last written time as a suffix.

@jackbrookes
Copy link
Member

jackbrookes commented Dec 14, 2022

This is a nice feature, but I think it should be an option, controlled by a boolean value in the FileSaver inspector.

Often I think overwriting is what is expected, if you do some setup step wrong and you restart the session you may not want all those old backup files cluttering things up.

However maybe its better to move the entire session folder if old files are detected on session start? e.g. the S001 folder could become S001_backup.

However, the old files are still somewhat unusable, because now the relative paths in the trial results are no longer valid, since all the names have changed. The only way to fix this would be to move the data to a sibling backup folder. Any thoughts?

@ahmed-shariff
Copy link
Contributor Author

I am of the opinion that no data should be discarded, even bad/incomplete data can be of use. Hence I opt having more files even though it might clutter the directory; that clutter doesn't bother me much with how I process data. But I see where you are coming from. I'll make that an option that can be toggled by the user.

I hadn't though of the relative paths in the trial_results.csv file, with that, as you say, the per file backup might not be the best option.
Also, wouldn't moving them to a sibling directory also have the same issue? as the base of the path would be changing.

One option is to use relative paths (relative to the trial_results.csv) in the trials_results.csv, but that most likely will be breaking anyone depending on that format downstream.

The other alternative is to move it to a backup directory without guarantees that the file-names in that backup would work without some processing?

As for the backup directory, what do you think about having a backup directory at StoragePath?
i.e., if a session is starting and the directory where the session is to be saved exists (e.g., S001), the directory would be moved to the backup directory. e.g., to <StoragePath>/backup/<experiemnt>/<ppid>/S001 from <StoragePath>/<experiemnt>/<ppid>/S001

@jackbrookes
Copy link
Member

If you want to continue work on this, I have a few comments.

I think the file path suggested is correct, that way relative paths still work. I would suggest a simpler implementation though, which is moving the entire directory tree over when starting a new session which has a risk of overwriting. I think this could be done inside the SetUp method of FileSaver. This is actually nice, because if you perform a session with 15 trials, then overwrite with 10 trials, the other 5 trials' data will be backed up, but not stored in the new folder.

Additionally:

  • Let's have an inspector boolean field (true by default) that allows you to turn of backup before overwriting. Personally I would find it annoying in some cases where file size is huge and I do want to overwrite
  • Make the backup folder name something like "_UXF_Backup" so its distinguished from experiment folders, but this string can be an inspector value too so it can be changed.

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

2 participants