-
Notifications
You must be signed in to change notification settings - Fork 13
Bug #630 population folder #648
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
Conversation
jan-petr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct reorganization of the code. But it still creates the Population file in the derivatives folder. Was that the problem - created on the correct location but too early? @BeatrizPadrela
Anyway - I removed the initFileSystem function from ExploreASL_Initialize and moved it only to structural/asl/population module. So this should be now fixed, but Henk should check as well...
HenkMutsaerts
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you're right, the idea is that the population and its subdirs are only created when needed. So this fix is nice, but even nicer to remove this folder creation out of general files and manage this within the individual wrappers that need these folders to exist.
The issue was not xASL_init_FileSystem but xASL_init_DefinePaths I believe, that creates everything upon initialization. Easiest fix for now is to move the contents of xASL_init_DefinePaths from line 81 onward (loop of xASL_adm_CreateDir) to the start of ExploreASL_ProcessMaster.
When data is loaded, it should define the paths, when data is processed (i.e., running ExploreASL_ProcessMaster) it should create the folders. Is this the case now, that ExploreASL_ProcessMaster is only run when processing is set to true, and this creates the folders?
I agree in principle. Anyway - the things already seem to be done according to what we want... Can we merge? |
HenkMutsaerts
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes this looks good. If this is tested and works, I'm good and can be merged. I would keep xASL_init_FileSystem in the modules for now to keep modularity.
d9674c5 to
ee81afd
Compare
Linked issue
Check out #630