TEST: Adding random generator with seed to icm tests

[pjsjongsung]

This commit adds seeds to test_square_iter and test_icm_square to stabilize the tests. square_gauss and square_1, which were originally global variables are now loaded through a function which has a numpy random generator inside with fixed seed. The variables were wrapped in a function because 1. there is no need for the variables to be global 2. this complies with a future PR that will add decorators on tests that require seed values.

Note that this is a separate PR created from a discussion in #2929 for immediate increase in stability of the tests while a PR to change all random functions in DIPY to the recent numpy convention is underway.

[skoudoro]

Looks good to me. Thanks @pjsjongsung

1. there is no need for the variables to be global

I agree with this statement, I do not know why they were global. Actually, I should check the whole test codebase to reduce this.

Waiting for the CI's to finish and if it is ok, I will go ahead.

Note that this is a separate PR created from a discussion in #2929 for immediate increase in stability of the tests while a PR to change all random functions in DIPY to the recent numpy convention is underway.

Instead of a big PR, maybe you should do it step by step, small PRs by small PRs. You could create an issue to identify all np.random that needs to be replace in multiple PR.
But if you feel you do it all in once, please, feel free to go ahead.

[pjsjongsung]

Thank you @skoudoro

I think I will push one PR because even though the changes are in multiple files, it is just a few lines per file. I feel like it will create too much small PRs.

[jhlegarreta]

Thanks for doing this @pjsjongsung.

Instead of a big PR, maybe you should do it step by step, small PRs by small PRs. You could create an issue to identify all np.random that needs to be replace in multiple PR.
But if you feel you do it all in once, please, feel free to go ahead.

I don't think splitting this across multiple PRs is the best approach.

All methods that need to be changed can be identified and changed easily (automatically, or almost) I believe (despite involving a given amount of work), so best is to do all at once.

I think I will push one PR because even though the changes are in multiple files, it is just a few lines per file. I feel like it will create too much small PRs.

👍.

[jhlegarreta]

@pjsjongsung one thing I missed: please use the appropriate commit message subject codes.

[pjsjongsung]

Sorry I missed that. I will make sure to do that in the later commits.

[jhlegarreta]

@pjsjongsung one thing I missed: please use the appropriate commit message subject codes.

@pjsjongsung not only the PR title, but the commit itself: you'll need to:

$ git commit --amend
(change the commit message subject)
$ git push -f origin icm_test_stable

[pjsjongsung]

Oh I was afraid it might disrupt the commit. Done! Thanks

[codecov]

Codecov Report

Merging #2938 (35253cd) into master (510e079) will increase coverage by 0.39%.
Report is 201 commits behind head on master.
The diff coverage is n/a.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2938      +/-   ##
==========================================
+ Coverage   81.39%   81.79%   +0.39%     
==========================================
  Files         145      146       +1     
  Lines       20142    20401     +259     
  Branches     3215     3239      +24     
==========================================
+ Hits        16394    16686     +292     
+ Misses       2931     2898      -33     
  Partials      817      817              

see 24 files with indirect coverage changes

[skoudoro]

All green, great ! merging, thanks. @pjsjongsung and @jhlegarreta !