search

qiita-spots / qp-deblur

2 stars 7 forks source link

Silva 12.8 #46

Closed sjanssen2 closed 6 years ago

sjanssen2 commented 6 years ago

support for Silva 12.8 reference

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.04%) to 99.479% when pulling 8c9cdc356b154f47102eb981d9685c81eb6e55c8 on sjanssen2:silva_12.8 into 184d583970a11d0b14674324a0961c65e488d9b4 on qiita-spots:master.

sjanssen2 commented 6 years ago

ready for reviews @antgonza @josenavas This is adding support for Silva as a non default reference tree into which deblur sequences can be placed.

antgonza commented 6 years ago

Code looks good, the only question is: should it be better that the references are discoverable? For example, that during init.py the plugin "searches" for files in support_files/sepp/reference/ and any foder in there (with that name) becomes an option? This will mean that in the future to add a new reference we only need to create and populate that folder. What do you think?

sjanssen2 commented 6 years ago

@antgonza: that sounds like a nice mechanism. However, it is not that easy to only place files in there. 1) Siavash did a heavy compute on Exceed to create the reference tree/alignment/info from Silva and they get distributed with the underlying "fragment-insertion" conda package 2) I had to generate "rename" and "placement" templates from those three files.

Since the above process includes all those manual steps, the additional burden of extending the if condition is not too big and it safeguards us from adding wrong files or with wrong filenames / content.

All in all, adding another reference is not easy :-/

antgonza commented 6 years ago

Well, one option is to place all those files within the same folder, no? Now, perhaps it's worth exploring (as now we have 2 examples)how to merge (and even test) those rename/place templates. What do you think? Also, what will it take?

antgonza commented 6 years ago

closing due to inactivity and lack of benchmarks with Silva ...