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] Installation issues, suggested code reorganization #2

Closed 0todd0000 closed 3 years ago

0todd0000 commented 3 years ago

This issue highlights a few different issues related to installation. I am submitting this as one issue because they are all related, and I've suggested one solution below that would solve all of these issues.



Minor issue: Installation instructions

Minor issue: Unneeded directories added to path

./fctSPM
./fctSPM/.git
./fctSPM/.git/branches
./fctSPM/.git/hooks
./fctSPM/.git/info
./fctSPM/.git/logs
./fctSPM/.git/logs/refs
./fctSPM/.git/logs/refs/heads
./fctSPM/.git/logs/refs/remotes
./fctSPM/.git/logs/refs/remotes/origin
./fctSPM/.git/objects
./fctSPM/.git/objects/info
./fctSPM/.git/objects/pack
./fctSPM/.git/refs
./fctSPM/.git/refs/heads
./fctSPM/.git/refs/remotes
./fctSPM/.git/refs/remotes/origin
./fctSPM/.git/refs/tags
./fctSPM/Article@JOSS
./fctSPM/Examples
./fctSPM/Figures
./fctSPM/mainFunctions
./fctSPM/plotFunctions
./fctSPM/spm1d_Pataky
./fctSPM/spm1d_Pataky/examples
./fctSPM/spm1d_Pataky/examples/nonparam
./fctSPM/spm1d_Pataky/examples/nonparam/0d
./fctSPM/spm1d_Pataky/examples/nonparam/1d
./fctSPM/spm1d_Pataky/examples/normality
./fctSPM/spm1d_Pataky/examples/normality/0d
./fctSPM/spm1d_Pataky/examples/normality/1d
./fctSPM/spm1d_Pataky/examples/stats0d
./fctSPM/spm1d_Pataky/examples/stats1d
./fctSPM/spm1d_Pataky/examples/stats1d_roi
./fctSPM/spm1d_Pataky/examples/stats2d
./fctSPM/spm1d_Pataky/spm8
./fctSPM/utilityFunctions
./fctSPM/utilityFunctions/cbrewer



Major issue: DO NOT ADJUST USER PATHS The current scripts in Examples call the addAbovePath.m function. This is highly problematic for two main reasons:

  1. addAbovePath.m is undocumented.
  2. Please never alter users' paths without their knowledge and explicit permission.




To solve these problems I suggest the following:

Currently the directory structure is:

./Article@JOSS
./Examples
./Figures
LICENSE
./mainFunctions
./plotFunctions
README
./spm1d_Pataky
./utilityFunctions

I suggest reorganizing as follows:

./Article@JOSS
./Examples
LICENSE
README
./src/    % contains contents of "mainFunctions"
./src/plot    % contains contents of "plotFunctions"
./src/util    % contains contents of "utilFunctions"
./src/spm1d_Pataky

With this organization it will be easy to explain installation, and to solve all other problems mentioned in this issue. Installation could be explained using, for example:

addpath( genpath( "./fctSPM/src" ) )
tramarobin commented 3 years ago

The folder was reorganized as recommended and the addAbovePath was deleted. However, can I still let the a line addpath(genpath("../src")) in the example scripts to add only the source code path or is this prohibited ? I still wrote how to add only the required path in the installation instructions.

0todd0000 commented 3 years ago

Please do not alter the path in scripts. Paths are relevant only to installation.

If the package is not installed correctly, then the scripts should not execute. When the package is correctly installed, then the scripts should execute properly.

Please do not mix installation with demo scripts.

tramarobin commented 3 years ago

The line with the installation procedure was deleted from all the examples.

0todd0000 commented 3 years ago

Ok, thank you!