tramarobin / fctSnPM

Using spm1d package (v.0.4.3), compute anova and post-hoc tests from anova1 to anova3rm, with a non-parametric approach (permutation tests)
GNU General Public License v3.0
2 stars 2 forks source link

JOSS Review: Simplify examples #20

Closed nicholst closed 3 years ago

nicholst commented 3 years ago

Hi,

As part of my JOSS review, I ran the suggested example in the READM.md but was frustrated when I saw thhat I had to edit some paths, not right at the top of the file.

The simple changes seems to be to supply default paths so they run without edits... i.e. I changed the [] empty paths to *_results where * is the name of the example script.

tramarobin commented 3 years ago

Thank you for this pull request.

As I agree it may be easier to pre indicate a savedirectory, it was discussed before with @0todd0000 in issue #6 and decided to let the user chose manually by default.

But as I understood, the part that was confusing is the creation of a Results folder. If indicated as you did by default, the name of the created repertory is easier to understand.

not right at the top of the file.

Also, I can change the location of these lines of code so that they are at the beginning of the script, with the indication that the user can indicate his saving directory.

I don't know what the best solution could be.

nicholst commented 3 years ago

For demonstration scripts I don’t see the need to make the user do anything. The goal is to minimize the friction for the user.

But if it’s been discussed before I’m not going to insist on it. If you don’t take on my suggestions consider:

  1. Empty sting ‘’ is a better null array [] in this setting
  2. Consider whether the savedir2 is needed… it refers to commented-out code.. maybe define it with the commented-out code or just use the same savedir variable.
  3. Except regarding the precious comments on savedir2, yes, moving it to the top of the file will be better.
0todd0000 commented 3 years ago

My major concern in issue #6 was that the demo script created directories and files, without the user's knowledge (unless they read the script in some detail), and also in locations that are not immediately obvious. I think the user needs to be aware that (a) the demo scripts create files and directories, and (b) the directory locations. On option is a dialog. But I agree with @nicholst that full automation and zero activity might be better.

I think we discussed this before, but is it possible to generate figures rather than files as default in these demo scripts?

tramarobin commented 3 years ago

I modified the examples to be in line with your suggestions.

Empty string was used as default savedir and savedir2 was defined as the same as savedir These lines of code were moved to the top of the files. In addition, a comment explicits that the function will create files in a new folder at savedir.

% Don't forget to add the source code path
% This funtion will automatically create multiple files in a save directory located at the savedir
% adress

savedir=''; % is the adress where the output of fctSnPM is saved 
savedir2=savedir; % is the adress where the output of saveNplot is saved 

A new fonction called onlyPlot was added to plot a part of the analysis obtain with fctSPM and fctSPMS. In 1D, the ANOVA and the means with SPM analysis are displayed. In 2D, the ANOVA and the post-hoc tests are displayed. The description of this function wad added to the optional functions documentation

nicholst commented 3 years ago

Super, looks good!

Out of curiosity, if the scripts are used as is, what happens? Is an error thrown or are the files created in the PWD?

tramarobin commented 3 years ago

Out of curiosity, if the scripts are used as is, what happens? Is an error thrown or are the files created in the PWD?

A dialog box opens to ask the user where to save the files.

nicholst commented 3 years ago

Ahhh! So sorry... had no idea that something sensible like that would happen.

On Thu, Jun 10, 2021 at 11:03 AM tramarobin @.***> wrote:

Out of curiosity, if the scripts are used as is, what happens? Is an error thrown or are the files created in the PWD?

A dialog box opens to ask the user where to save the files.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tramarobin/fctSPM/pull/20#issuecomment-858489534, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABHKYQ3AR2GRKXOHICB6KE3TSCEVJANCNFSM46KI6LFA .

tramarobin commented 3 years ago

This was one of the options considered to let the user choose a savedir by default. But with a modification possible at the beginning of the script, it should be easier to complete the string array and automate the script.

tramarobin commented 3 years ago

@nicholst, as different modifications has been made, I close this pull request.