prefer use of new-style NumPy random API in tests

[grlee77]

Description

This PR follows up on #5357, mainly removing residual use of np.random.RandomState from test classes. This changes here are mostly trivial, but I have a couple specific points that may be worth considering:

1.) Regarding segmentation.slice and segmentation.quickshift. For those two, there are a lot of tests which check specific label values as below:

scikit-image/skimage/segmentation/tests/test_slic.py

Lines 28 to 31 in e4cc469

	assert_equal(seg[:10, :10], 0)
	assert_equal(seg[10:, :10], 2)
	assert_equal(seg[:10, 10:], 1)
	assert_equal(seg[10:, 10:], 3)

Is it really important to check that the specific values occur in this order? If not, we could replace with something like just verifying there is only a single, unique label within each region (The correct overall number of labels is already checked separately)

assert np.unique(seg[:10, :10]).size == 1

etc.

2.) There is one residual use of RandomState used when performing slic with a mask.

scikit-image/skimage/segmentation/slic_superpixels.py

Line 38 in e4cc469

rnd = random.RandomState(123)

There are a couple of test results that seem to assume the existing initial seed points based on the current RandomState seed, so if we really want to remove that we have to update those test cases with new expected results. I just added a comment to that effect rather than switching it here. Similarly for a specific seed used to compare to precomputed results in one of the unsupervised_wiener test cases.

Checklist

• Docstrings for all functions
• Gallery example in ./doc/examples (new features only)
• Benchmark in ./benchmarks, if your changes aren't covered by an
  existing benchmark
• Unit tests
• Clean style in the spirit of PEP8

For reviewers

• Check that the PR title is short, concise, and will make sense 1 year
  later.
• Check that new functions are imported in corresponding __init__.py.
• Check that new features, API changes, and deprecations are mentioned in
  doc/release/release_dev.rst.

[pep8speaks]

Hello @grlee77! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-08-31 02:47:48 UTC

[grlee77]

This PR currently is only for modernizing the random call styles in the tests and does not make any API or behavior changes. I think we can address unifying random_state vs. random_seed etc. in a separate PR.

[mkcor]

1. Is it really important to check that the specific values occur in this order?

No, indeed, since the order of labels as 'integers' (which they are not) doesn't bear any specific meaning. By the way, I'm in favour of closing this issue: #5049

If not, we could replace with something like just verifying there is only a single, unique label within each region.

Absolutely!

(The correct overall number of labels is already checked separately)

All good then.

[mkcor]

@grlee77 the merge conflict here was expected from merging #5220.

[mkcor]

2. There are a couple of test results that seem to assume the existing initial seed points based on the current RandomState seed, so if we really want to remove that we have to update those test cases with new expected results. I just added a comment to that effect rather than switching it here.

Oh, iirc #5357 did that (updating expected results) for a few tests. Ok, let's leave these remaining pieces to a subsequent PR.

[grlee77]

Oh, iirc #5357 did that (updating expected results) for a few tests. Ok, let's leave these remaining pieces to a subsequent PR.

Perhaps a bigger concern in the case of slic is whether downstream projects or users were using a specific seed when calling slic. If we change to the new generator within that function, then they would suddenly get a different initialization. I don't think the old interface is going away, so I wasn't sure it was worth updating in this case.

[alexdesiqueira]

LGTM; thanks @grlee77!

[mkcor]

Why this change, just out of curiosity? 🙂