ENH: Adds seed parameter to rarefy

[VinzentRisch]

closes #55

• Adds seed parameter to subsample-ids function.
• Seed is per default set to 1. It can be set to "random" what creates a random 32 bit int as the seed.
• Setting the seed to default 1 was done to ensure that the seed is logged in provenance.
• At this point it is not possible to have a random seed that is created in the function that will be recorded in provenance.

[gregcaporaso]

@VinzentRisch, I completely understand the motivation for this, but I don't think this solution will work out as-is. The vast majority of users wouldn't change this setting from the default, which would have the effect of there being no randomization of the seed, and repeat runs would use the same seed.

At the moment, we don't have a way to store the seed in provenance, but we know that this is something that should be addressed. What I would recommend is that you make the default "random", and if you want to make sure your seed is stored in provenance, you can generate a random in outside of QIIME 2 and pass that in. Do you want to make that change, and then ping me for re-review? Or let me know if you'd like to discuss more.

[gregcaporaso]

See comments on this PR and on #317

[VinzentRisch]

Okay that makes sense. I'll make the changes.

[VinzentRisch]

Hi @gregcaporaso
I made the changes.

[gregcaporaso]

@VinzentRisch, thanks for the edits. I have additional specific suggestions on this implementation.

[VinzentRisch]

Hi @gregcaporaso
I'm sorry about the last changes. But now it should be correct. You can just set the seed parameter of subsample to None, that creates a random seed internally in the function. I also implemented your tests with the iteration.
There are some tests for the heat maps failing in the CI, I don't really know what the problem is.

[gregcaporaso]

Thanks @VinzentRisch - I will review this week.

[gregcaporaso]

Thanks for the updates @VinzentRisch. I tested this locally and it's working for me. This is ready for merge, pending the failing heatmap tests.

I'm not sure what's going on with the failing heatmap tests, but I don't think it's anything that you're doing. I'll discuss with the folks on my end to figure out what's going on and then get this merged.

[gregcaporaso]

@VinzentRisch, do you have an option to "Update branch" on this pull request, as described here. If so, could you do that? I want to see if it addresses the test failures.

[VinzentRisch]

I updated the branch, but the tests are still failing.

[gregcaporaso]

Thanks @VinzentRisch, we'll look into it from here. I don't think it's anything that you're doing wrong.

[gregcaporaso]

Alright, we got it sorted out @VinzentRisch - merged. Thanks again for the work on this one, and for your patience with the back-and-forth!