-
Notifications
You must be signed in to change notification settings - Fork 13
Closes #892 ExploreASL_Initialize: rm paths before addpath #894
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.
OK
|
@jan-petr & @HenkMutsaerts: did some changes to reduce the crazy verbosity, I bet users would greatly complain about that. I also added a condition for matlab related toolboxes, because that leads to super annoying bugs otherwise. Edit: please add the "how to test" section next time, we kind of agreed on this. |
|
@HenkMutsaerts & @jan-petr: For me all of this is asking for trouble. I got a ton of warnings and some errors afterwards just during coding, because Matlab was complaining about missing paths. I definitely wouldn't merge it like this. If a user decides to put spm inside the internal matlab directory, something is wrong on their side on not in ExploreASL. Especially if multiple users share a license they get like 100 warnings every time they initialize. That's crazy. Maybe you don't see it on your side, but that's not tested properly. |
OK then it needs to be made more stable, such that it will only throw away important paths. Or we can for now include only spm, that should be sufficient as CAT12 & LST are usually in spm paths, and fsl is dealt with separately. Let me add a change and can you test it then again Michael? |
Sure. I guess we should just change our general method. Instead of removing everything besides ExploreASL, we should remove only the things that could be troublesome, so all paths besides the ExploreASL ones that include our internal toolboxes. Meaning we only throw out other xASL, spm, bids-matlab, cat, and lst related paths, right? |
@MichaelStritt
|
262b547 to
f451816
Compare
Linked issue
Closes #892
How to test
Required: if not defined in the linked issue, add a simple test description here
Comments
Optional: add helpful comments for the reviewers here