search

biocore / metagenomics_pooling_notebook

Jupyter notebooks to assist with sample processing
MIT License
8 stars 16 forks source link

Port Mackenzie's NPH_calc_df_SLURRY.ipynb functionality into modules and unit-test #155

Closed AmandaBirmingham closed 7 months ago

AmandaBirmingham commented 7 months ago

Also includes slight updates to the README.md to reflect the fact that a Qiita install is required to run unit tests.

charles-cowart commented 7 months ago

@AmandaBirmingham @mmbryant23 it looks like some of the tests are still failing - because this is dev branch, are these failing tests due to issues beyond our current control and therefore should this PR be merged as-is? Or should we wait until they are fixed?

AmandaBirmingham commented 7 months ago

In general I'd say let those tests fail as they are unrelated to the stuff being pushed in this PR. However, I see that at least a couple of them are from the earlier implementation of the synDNA port, which now lives somewhere else anyway, so i removed those modules and tests.

Note that I have NOT yet removed the data files associated with the tests, because I want to double-check they aren't referenced by anything else before I do so.

charles-cowart commented 7 months ago

In general I'd say let those tests fail as they are unrelated to the stuff being pushed in this PR. However, I see that at least a couple of them are from the earlier implementation of the synDNA port, which now lives somewhere else anyway, so i removed those modules and tests.

Note that I have NOT yet removed the data files associated with the tests, because I want to double-check they aren't referenced by anything else before I do so.

Okay, that sounds reasonable. I also confirmed that the current dev branch failed testing so it's not like we're going south on this branch.

charles-cowart commented 7 months ago

@AmandaBirmingham @mmbryant23 okay guys we're merged yay! :) Just a quick reminder to git pull back to your fork and local branch before continuing. I think this is the first PR we've merged since starting this.