Repository navigation
Update [Notebook] : Add bypass function to directly download outputsfrom execution name + add check on each step to verify needed files are here - #59
Conversation
…from execution name + add check on each step to verify needed files are here
…cks, clean diffs and idempotence to freesurfer notebook
| " if f.is_dir():\n", | ||
| " flatten_folder(Path(f))" | ||
| "session_name = restore_session_from_output_dir(FS_CROSS_OUTPUT_DIR) # Replace with a specific session name if needed\n", | ||
| "download_by_session_name(session_name, str(FS_CROSS_OUTPUT_DIR))\n", |
There was a problem hiding this comment.
is it to avoid downloading files already present ?
does not session.download_outputs handle that ?
There was a problem hiding this comment.
Yes, it avoid re-downloading by checking existence and file size. It also avoids the creation of the VIP timestamp folder that I had to flatten before.
session.download_outputs does try to check for existing files but include the timestamp folder in the path, which I don't have if it's a rerun (flattened). Other than that it has the download size check I added in the new download functions, but in my download_by_session_name function I also do the VIP expected size check pre-download to filter files.
There was a problem hiding this comment.
i think the download though vip with also benefit from the new download checks added in vip.py.
if i'm not mistaking, the only benefit here is to "flatten" / download without the timestamp folder.
would it not be possible to add this option in session.download_outputs
There was a problem hiding this comment.
Removed custom download_by_session_name and added direct_download_path field to the download functions
| "for f in FS_CROSS_OUTPUT_DIR.iterdir():\n", | ||
| " if f.is_dir():\n", | ||
| " flatten_folder(Path(f))" | ||
| "session_name = restore_session_from_output_dir(FS_CROSS_OUTPUT_DIR) # Replace with a specific session name if needed\n", |
There was a problem hiding this comment.
can there be several session_name in 1 output folder ?
There was a problem hiding this comment.
No there can be only one, as there is only one session_data.json per output folder
| "# Since results may arrive in a VipSession folder (2026-X), when it's the case we flatten this folder to put files directly under FS_CROSS_OUTPUT_DIR\n", | ||
| "for f in FS_CROSS_OUTPUT_DIR.iterdir():\n", | ||
| " if f.is_dir():\n", | ||
| " flatten_folder(Path(f))" |
There was a problem hiding this comment.
i think it is better to not do that, but does it not change something when we when to list the tar.gz latter ?
Or does the new download_by_session_name method change that ?
There was a problem hiding this comment.
Was removed by previous commit since we now directly download files inside output folder and not through VIP timestamp folder, so we don't need the flatten anymore.
For the tar.gz we need the files to be directly under the output folder to use FS_CROSS_OUTPUT_DIR.iterdir().
|
|
||
| #----------------------------------------------------------------------------- | ||
| def remove_local_file(path): | ||
| if path is None: |
There was a problem hiding this comment.
maybe verify that os.path.exists(path) ?
| " if f.is_dir():\n", | ||
| " flatten_folder(Path(f))" | ||
| "session_name = restore_session_from_output_dir(FS_CROSS_OUTPUT_DIR) # Replace with a specific session name if needed\n", | ||
| "download_by_session_name(session_name, str(FS_CROSS_OUTPUT_DIR))\n", |
There was a problem hiding this comment.
i think the download though vip with also benefit from the new download checks added in vip.py.
if i'm not mistaking, the only benefit here is to "flatten" / download without the timestamp folder.
would it not be possible to add this option in session.download_outputs
| " missing = expected_paths - actual_paths\n", | ||
| " print(f\" → {out_tar.name} exists but missing {len(missing)} files, regenerating.\")\n", | ||
| " except (tarfile.TarError, OSError):\n", | ||
| " print(f\" → {out_tar.name} exists but is corrupted, regenerating.\")\n", |
There was a problem hiding this comment.
that works, but i wonder if using some timestamps like just above would not be simpler.
if any of the extracted dir of the subject is newer that the tar timestamp, then re-do tar.
would that work ?
There was a problem hiding this comment.
It would, added the check
…ssion download_outputs function to handle direct path download + add/improve checks
No description provided.