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

Silva 2019 #63

Closed
wants to merge 29 commits into from
Closed

Silva 2019 #63

wants to merge 29 commits into from

Conversation

sjanssen2
Copy link
Collaborator

Since the original PR #32 is quite old, I am starting a new one here to declutter the introduced changes:

Necessary changed to support non default references (which is Greengenes 13.8) like Silva 12.8.
I needed to add another parameter which is a filepath to an "info" file, which holds information about tree construction via RAxML from the multiple sequence alignment. This is used by pplacer to determine correct branch length for insertions.
This PR is solving the technical issues of #21 However, it neither compiles current Silva reference files nor hosts them somewhere.

There are related posts in the forum.qiime2.org:
https://forum.qiime2.org/t/making-rooted-silva-tree/11011/9
https://forum.qiime2.org/t/suggestion-silva-v132-for-q2-fragment-insertion/7844/5

@sjanssen2 sjanssen2 mentioned this pull request Sep 4, 2019
@ebolyen ebolyen mentioned this pull request Sep 4, 2019
@sjanssen2
Copy link
Collaborator Author

Hi @thermokarst,

I thought about your comment to stop vendoring data #66 and still think about a good way to distribute multiple reference packages for SEPP (Greengenes 13.8 and Silva 12.8 at the moment). Thus, I created two bioconda packages (sepp-refgg138 and sepp-refsilva128) consisting only of those 4 and 3 files respectively.

The only issue is, that the non default reference package (silva) needs to be qza'ed by the user before they can use it - which might represent a severe obstacle. Thus, the build.sh script of q2-fragment-insertion imports those files into qza's. Therefore, sepp-refsilva128 is a build but not a run dependencies - whereas sepp-refgg138 is needed for build and run.
Hope this solves all our issues.

@sjanssen2
Copy link
Collaborator Author

ping @ebolyen @thermokarst any comments? I'd like to provide Silva and other alternative references soon. Can you give me a rough estimate of a time line? What are the chances to get shipped with qiime-2019.10 ?
Thanks!

@ebolyen
Copy link
Member

ebolyen commented Sep 16, 2019

Hey @sjanssen2! Sorry for the delay, @thermokarst has been busy with a grant related project, I'm sure he'll poke his head in once he has some breathing room.

Our goal as I understand it is to get this into 2019.10, I think @thermokarst has some work in progress towards that end, but I'm not certain of the details.

@thermokarst thermokarst self-assigned this Sep 16, 2019
@thermokarst
Copy link
Contributor

Hi @thermokarst,

I thought about your comment to stop vendoring data #66 and still think about a good way to distribute multiple reference packages for SEPP (Greengenes 13.8 and Silva 12.8 at the moment). Thus, I created two bioconda packages (sepp-refgg138 and sepp-refsilva128) consisting only of those 4 and 3 files respectively.

The only issue is, that the non default reference package (silva) needs to be qza'ed by the user before they can use it - which might represent a severe obstacle. Thus, the build.sh script of q2-fragment-insertion imports those files into qza's. Therefore, sepp-refsilva128 is a build but not a run dependencies - whereas sepp-refgg138 is needed for build and run.
Hope this solves all our issues.

Hey @sjanssen2! How about we just publish data.qiime2.org links to QZAs for these databases, and stick them up at https://docs.qiime2.org/2019.7/data-resources/? I would prefer not to couple these databases with the plugin, since this can cause a pretty significant bottleneck for use installs, integration testing, etc. As @ebolyen mentioned, I am working on a time-sensitive project right now, and will be preoccupied for the next week or so. Thanks!

@thermokarst thermokarst removed their assignment Oct 10, 2019
@sjanssen2 sjanssen2 mentioned this pull request Oct 29, 2019
6 tasks
@thermokarst
Copy link
Contributor

Closing in favor of #66

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