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

Unexpected behaviour: CONFIG file is modified by the software #86

Open
jcohenadad opened this issue Apr 14, 2024 · 7 comments
Open

Unexpected behaviour: CONFIG file is modified by the software #86

jcohenadad opened this issue Apr 14, 2024 · 7 comments
Assignees

Comments

@jcohenadad
Copy link
Member

After figuring out #85, I manually created a YML file that looked like this:

FILES_LABEL:
- sub-Marseille1_UNIT1.nii.gz
- sub-MGH1_UNIT1.nii.gz
- sub-MNI1_UNIT1.nii.gz
- sub-MPI1_UNIT1.nii.gz
- sub-NTNU1_UNIT1.nii.gz
- sub-NYU1_UNIT1.nii.gz
- sub-UCL1_UNIT1.nii.gz

After running manual_correction.py, I wanted to document in another project how I did it. So I was about to package my config file, but... what a surprised: it has been modified by the software. It now looks like this:

CORR_LABEL:
- sub-Marseille1_UNIT1.nii.gz
- sub-MGH1_UNIT1.nii.gz
- sub-MNI1_UNIT1.nii.gz
- sub-MPI1_UNIT1.nii.gz
- sub-NTNU1_UNIT1.nii.gz
- sub-NYU1_UNIT1.nii.gz
- sub-UCL1_UNIT1.nii.gz
FILES_LABEL: []

Modifying config file is highly unexpected and should not happen.

@valosekj
Copy link
Member

Thanks for reporting this. The modification of the YML config file was introduced in #57 to track which files have been modified. It is useful, for example, when a user needs to pause/restart corrections. The correction process in such a case continues from the last non-corrected subject instead of starting from the beginning.

But I agree that it might be confusing. I have an open issue about it: #62

Relevant comment from Nathan here: #57 (comment)

@jcohenadad
Copy link
Member Author

jcohenadad commented Apr 14, 2024

Maybe a <CONFIG>.tmp should be created? Other ideas welcome.

Also, this behaviour should appear as a subsection in the README. In general (bigger issue), the README section should be self-contained, and users should not have to go to the example section to understand how to use this software.

@valosekj valosekj self-assigned this Apr 15, 2024
@valosekj
Copy link
Member

Potential idea:

The original config file would be kept as it is (without any filename or content modifications), for example, config.yml:

FILES_LABEL:
- sub-Marseille1_UNIT1.nii.gz
- sub-MGH1_UNIT1.nii.gz
- sub-MNI1_UNIT1.nii.gz
- sub-MPI1_UNIT1.nii.gz
- sub-NTNU1_UNIT1.nii.gz
- sub-NYU1_UNIT1.nii.gz
- sub-UCL1_UNIT1.nii.gz

After running and stopping manual_correction.py, a new copy of the config file tracking the corrected files would be created, for example, <CONFIG>.tmp:

CORR_LABEL:
- sub-Marseille1_UNIT1.nii.gz
- sub-MGH1_UNIT1.nii.gz
- sub-MNI1_UNIT1.nii.gz
FILES_LABEL:
- sub-MPI1_UNIT1.nii.gz
- sub-NTNU1_UNIT1.nii.gz
- sub-NYU1_UNIT1.nii.gz
- sub-UCL1_UNIT1.nii.gz

If the user wanted to continue from the last modified file, they would enter the path to the <CONFIG>.tmp file.
At the same time manual_correction.py will contain a condition to overwrite <CONFIG>.tmp file (and not create <CONFIG>.tmp.tmp) when rerunning the script.

@jcohenadad
Copy link
Member Author

@maxradx please chip in and cross-ref your approach

@maxradx
Copy link

maxradx commented May 28, 2024

Thanks for the follow-up.

  1. The image below summarizes my approach that I use in 3DSlicer. An interesting point is that the GUI “Case list” allows you to have an eye on all cases of interest, and you can select the one that you want if the actual case is not the one that you want to be loaded.

  2. With the manual_correction script, maybe it could be a good idea to insert at the beginning of the script a function that creates a list of all cases (ex. filename or filepath) that are of interest for this segmentation task (e.g. a list of all file that could be saved in a “allCases.yaml”)? I am not sure how the manual_correction script loads the first remaining case, but another list could be created at the beginning (ex remainingCases), which is updated each time a segmentation is performed (removing the case from the list). At the next use, manual_correction could make loading automatically the first case of the list remainingCases?

image

image

@NathanMolinier
Copy link
Contributor

Thanks @maxradx for sharing you method but few things are missing to make it usable:

  • At the beginning your are talking about a config file but how do we generate this file ? I remember that there was a lot of variables, not only the contrast.
  • Did you create a new 3Dslicer module ? Is the code available somewhere ? How can I use it ?

@maxradx
Copy link

maxradx commented May 28, 2024

1- Yes there are a lot of variables but they could be listed and the code adapted (quite easily I think). The config file comes with the module in 3DSlicer developed by Dr Letourneau. Using manual_correction, we could create a function at the beginning of the script so the first time it is ran, a generic config file is generated in a specified folder and then we can customize it for further use?
2- For the moment, all that I ve made is based from Dr Letourneau 3DSlicer's module, which is private I think (this is the branch of my code : https://github.com/laurentletg/ICH_SEGMENTER_V2/tree/testing_mb). Also, for the moment, I have not finished what I want to show and did not clean my code (still experimental, very very very unclean and not ready to be used). I will let you know once the code will be ready!

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

4 participants