Skip to content

Conversation

@MichaelStritt
Copy link
Contributor

Linked issue

Check out #630

@MichaelStritt MichaelStritt added the bug Something isn't working label Jun 14, 2021
@MichaelStritt MichaelStritt self-assigned this Jun 14, 2021
@MichaelStritt MichaelStritt requested a review from jan-petr June 14, 2021 13:02
@MichaelStritt MichaelStritt linked an issue Jun 14, 2021 that may be closed by this pull request
Copy link
Contributor

@jan-petr jan-petr left a 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...

@jan-petr jan-petr requested a review from HenkMutsaerts June 14, 2021 18:33
Copy link
Member

@HenkMutsaerts HenkMutsaerts left a 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?

@jan-petr
Copy link
Contributor

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. xASL_init_DefinePaths should not create anything - but this is already fixed by Michael. All directory creation is done in xASL_init_FileSystem - this is then called from all modules (ASL, structural, etc.). So it is now indeed only run when the processind modules are run. It might be possible to move all this from these modules to ExploreASL_ProcessMaster if needed - but not sure if this wouldn't have any complications.

Anyway - the things already seem to be done according to what we want... Can we merge?

@jan-petr jan-petr requested a review from HenkMutsaerts June 15, 2021 10:24
Copy link
Member

@HenkMutsaerts HenkMutsaerts left a 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.

@jan-petr jan-petr assigned jan-petr and unassigned MichaelStritt Jun 17, 2021
@jan-petr jan-petr force-pushed the bug-#630_PopulationFolder branch from d9674c5 to ee81afd Compare June 17, 2021 11:26
@jan-petr jan-petr merged commit ee81afd into develop Jun 17, 2021
@jan-petr jan-petr deleted the bug-#630_PopulationFolder branch June 17, 2021 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Population folder created within BIDS2Legacy

4 participants