[DOC] Fixes Random Seed Help Text #281
Conversation
|
I know we like to leave good first issues open, but that one's been open for a while at this point. Can close if maintainers disagree. |
Codecov Report
@@ Coverage Diff @@
## master #281 +/- ##
=======================================
Coverage 46.16% 46.16%
=======================================
Files 35 35
Lines 2060 2060
=======================================
Hits 951 951
Misses 1109 1109
Continue to review full report at Codecov.
|
tedana/workflows/tedana.py
Outdated
| @@ -140,7 +140,7 @@ def _get_parser(): | |||
| type=int, | |||
| help=('Value passed to repr(mdp.numx_rand.seed()) ' | |||
| 'Set to an integer value for reproducible ICA results; ' | |||
| 'otherwise, set to -1 for varying results across calls.'), | |||
| 'otherwise, set to 42 for varying results across calls.'), | |||
There was a problem hiding this comment.
I think we actually want to say something like this
| 'otherwise, set to 42 for varying results across calls.'), | |
| 'otherwise, set to -1 for varying results across calls. ' | |
| 'Default value is 42.'), |
|
Thanks for addressing this ! One requested change and then I think this is good to merge 👍 |
|
Hm, I decided to read through the other help texts to make sure it conforms with them; most others don't list a default. It shouldn't go on this PR, but should I open an issue to go ahead and add defaults to the other options as well? |
|
I don't think we strongly need to list our defaults in the help text, since folks can usually access them from the function signature. But this is a bit of a special case, in my mind, since we've (already) confused folks as to what seed is generally used ! Happy to be argued that all arguments need defaults, but agree that's for another PR 😸 |
| @@ -140,7 +140,7 @@ def _get_parser(): | |||
| type=int, | |||
| help=('Value passed to repr(mdp.numx_rand.seed()) ' | |||
| 'Set to an integer value for reproducible ICA results; ' | |||
| 'otherwise, set to -1 for varying results across calls.'), | |||
There was a problem hiding this comment.
So sorry, I meant that the default should be given in addition to this note about the special meaning of -1. I think we need both pieces of information, here !
There was a problem hiding this comment.
Oh, duh, that makes sense. Let me make Travis work some more.
Closes #193 .
Changes proposed in this pull request: