-
Notifications
You must be signed in to change notification settings - Fork 11
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
Move to the Brainstorm plugin manager? #159
Comments
Hello Francois, Since the PR has been merged in Brainstorm, I think we can close this issue. One last question for you and @thomas-vincent, What should we do about the previous installation scheme? (eg nst_install script) ? Should I remove them from the current repository? Can you confirm me, also, that Nirstorm is currently included in the compiled version of Brainstorm? |
Maybe we could keep them and move them to a folder named "source_install".
They are still useful to install development-specific stuff.
…On Wed, Apr 14, 2021 at 2:50 PM Edouard Delaire ***@***.***> wrote:
Hello Francois,
Since the PR has been merged in Brainstorm, I think we can close this
issue.
One last question for you and @thomas-vincent
<https://github.com/thomas-vincent>, What should we do about the previous
installation scheme? (eg nst_install script) ? Should I remove them from
the current repository ?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#159 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAE2GDX3QQL4OBBIQLTYUY3TIXPXLANCNFSM4ZDVPZCQ>
.
|
This is not useful for Brainstorm, so I don't have any opinion.
It should. All the Nirstorm code is compiled and included in the package. There might be other issues, but since I don't work with NIRS data at all, it's difficult for me to test it. |
And close the issue when you validated that it works. |
One more problem to solve: the co-dependency of the nirstorm and brainentropy plugins.
Therefore
For both options: set |
Hello What do you think of this :
with nst_MEM_option.m
so Best is installed when you click on the option button. |
No: The GetDescription() functions are called often and must be as fast and as self-contained as possible. Calling external functions from there is not recommended, and installing plugins from it is impossible. If you want the two plugins nirstorm and brainentropy to be linked, this is exactly what |
nst_MEM_option is only called if you click on the button; no ? i tried to put a disp inside; and its not displayed if you don't click on the button. Meaning the function is not called |
ok. this might be the better option. it's just that not a lot of people will be using MEM when using Nirstorm. So i am not sure it's ok to 'force' everyone that is using nirstorm to also dowload Best; but guess that's ok :) Can I ask you to make this change in the brainstorm repo ? :) |
This is bad coding practice. Please chose between:
|
Second option with having a label asking the installation of an extra plugin is lighter. |
ok. I will use the second option. Should make the change latter today in the nirstorm repository. May i ask you why the proposed option is a bad coding practice ? |
Hello @ftadel, I just realized that when nirstorm is loaded, we cannot download any other plugin. What could be the reason? |
I'm trying to separate as much as possible:
The batch management should be something completely static, independent from the user's local installation. One goal is to be able to generate scripts that are sent to a distant computation node. Therefore it should not be the batch editor that installs the plugin, but the batch execution (either the calling script or the process itself in the Run() function). If you want to be really formal: brainentropy is a requirement for nirstorm. I'm not sure I managed to be very clear here... but this bothers me because this is the kind of non-scriptable behaviors we're trying to remove from the software. |
I can't reproduce this problem... |
Actually, it doesnt't work. it seems GetDescription is not called after Brainstorm is started. So if Best is not in the path when brainstorm is started then you cannot use Best so you need to load Best and then restart Brainstorm. Issue is also that if you quit matlab, Best is removed from the path. so Every-time, we need to open brainstorm, add Best, and restart Brainstorm ...
Also it seems that panel_brainentropy doesn't initialize everything |
You could make it load automatically just like nirstorm, by adding:
Maybe this is something that could be fixed in panel_brainentropy.m directly, to make it more self-contained? |
Cross-posting the issue from the Brainstorm repo for including all the developers:
brainstorm-tools/brainstorm3#388
The text was updated successfully, but these errors were encountered: