astropy: TST: Do not use random seed in doctest
See https://github.com/astropy/astropy/pull/10644#pullrequestreview-634247783 from @tupui
We did this (#10644) in SciPy and the only difference is that we removed all seeding from examples. This is to enforce best practices. Using small seeds is bad for the entropy of the generator and also common values like 0, 42, 12345 end up in every code which leads to bias in results. Seeing should only be used for testing.
Here I am proposing to remove the seed and just allow variation in the results to pass the doctests (in SciPy I can add # random
or # may vary
, I am not sure if this is standard or not. Maybe this is your # doctest: +FLOAT_CMP
?).
I can do a PR on this branch if you want some help. Also, I would suggest to add somewhere in the doc a paragraph about seeding. See what we did for example https://scipy.github.io/devdocs/tutorial/stats.html#random-number-generation.
About this issue
- Original URL
- State: open
- Created 3 years ago
- Reactions: 1
- Comments: 17 (7 by maintainers)
Ok thanks for the update! No problem, just let me know 😉
FYI, for another reference, I did this for scikit-image here https://github.com/scikit-image/scikit-image/pull/5357
I can see how this solution would be nice but it also introduce more “magic” in the doctest and might make things harder to debug in the future. On the other hand, using
testsetup
is more explicit but might introduce a lot of duplication across the docs.If indeed we want to use what you implemented for
scipy
, perhaps it should be inpytest-doctestplus
instead…? cc @saimntl;dr – I am not sure and I should not be making this decision on my own. Will need discussions. Thanks for your info!
FYI, we are still having lot of discussion on SciPy about how to do this in the best way. I propose to do a PR once we come to a decision on our side. This way I would have better arguments and possibly a better solution for you here.
@pllim I will be happy to propose a PR 😃