Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added default moltype for infernal #23

Merged
merged 1 commit into from
May 14, 2014

Conversation

gregcaporaso
Copy link
Contributor

this should have been part of #19

@gregcaporaso
Copy link
Contributor Author

Be sure to view this one with whitespace hidden when reviewing (this link will show you the diff with whitespace hidden).

@gregcaporaso
Copy link
Contributor Author

@josenavas, can you review this one? Need it for a QIIME PR.

@squirrelo
Copy link

Just out of curiosity, why DNA as default instead of RNA when the tool is built with RNA in mind?

@josenavas
Copy link
Member

The code looks good. As we now there is no way of testing this before merging, so we can merge, run it through QIIME and check that it didn't broke anything.

@gregcaporaso once you provide an answer to @squirrelo I'll merge this

@gregcaporaso
Copy link
Contributor Author

@squirrelo, it's because the only place that we're using brokit is QIIME, where we're passing DNA sequences in. (This whole package is kind of a necessary evil to remove the PyCogent dependency from QIIME.)

@gregcaporaso
Copy link
Contributor Author

Note that I tested locally and the change works.

@squirrelo
Copy link

Fair enough. This is just something that's going to come up when brokit is
phased out for whatever is next, so figured it would be good to bring up
now.

On Tue, May 13, 2014 at 10:43 PM, Greg Caporaso [email protected]:

@squirrelo https://github.com/squirrelo, it's because the only place
that we're using brokit is QIIME, where we're passing DNA sequences in.
(This whole package is kind of a necessary evil to remove the PyCogent
dependency from QIIME.)


Reply to this email directly or view it on GitHubhttps://github.com//pull/23#issuecomment-43041644
.

@josenavas
Copy link
Member

Thanks @gregcaporaso merging.

@squirrelo this repo should be used only on QIIME until we decide which app controllers do we actually want to include in skbio. Then, they'll be recoded....

josenavas added a commit that referenced this pull request May 14, 2014
added default moltype for infernal
@josenavas josenavas merged commit e7161eb into biocore:master May 14, 2014
@gregcaporaso
Copy link
Contributor Author

Thanks!

@gregcaporaso gregcaporaso deleted the infernal-fix branch May 14, 2014 04:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants