I’m just getting started on working with this to do a review. Overall this is looking really good so far! It’ll be nice to finally get this released!
I was able to install q2-ninja-ops in my QIIME 2 2018.2 environment, but so far I haven’t been able to install ninja-ops itself through conda. What channels are you relying on for this? I’m working on macOS, and I’m getting the following error:
$ conda install -c knights-lab ninja_ops
Solving environment: failed
PackagesNotFoundError: The following packages are not available from current channels:
If you’re not sure what’s going on with conda there, I can help out, but just didn’t want to start poking around if you could just give me a command that would work.
My first recommendation is that you expand the tests a little bit. At the moment, you’re testing that the actions run successfully, but there is no sanity test that the results look reasonable. When we’re wrapping other applications, we generally assume that the underlying application is sufficiently tested, and we just put some very minimal tests in place to ensure that the plugin isn’t mangling the results of the underlying application at all, and that any parameters that the user passes make their way into the underlying application. The
q2-vsearch tests are a good example of this - you can see the closed-reference tests here, and you can feel free to use any/all of the data from those tests.
A couple of other high-level recommendations:
- I recommend changing the name
cluster-sequences-closed-reference. Take a look at this tutorial - the cluster sequences would imply that it’s taking sequence data (not a feature table) as input. This change isn’t a requirement, but I think it would help users understand how the method fits into QIIME 2.
- You should add descriptions for all of the input/output/parameters in your two methods. See here for an example of how to do that. See here for an example of what that documentation will look like.
I can give some more feedback once I get ninja-ops installed with conda - again, just let me know if you need some help with that.