Skip to content
This repository has been archived by the owner on Nov 9, 2023. It is now read-only.

addition of LEA-Seq method #1514

Merged
merged 126 commits into from
Nov 12, 2014
Merged

addition of LEA-Seq method #1514

merged 126 commits into from
Nov 12, 2014

Conversation

charudatta-navare
Copy link
Contributor

Implements Low-Error Amplicon Sequencing (LEA-Seq) method, described in: Faith, Jeremiah J., et al. "The long-term stability of the human gut microbiota." Science 341.6141 (2013).
I need to add a couple of functions in split_libraries_lea_seq.py, and I need to add unit tests. I will add those soon.

Implements Low-Error Amplicon Sequencing (LEA-Seq) method, described in: Faith, Jeremiah J., et al. "The long-term stability of the human gut microbiota." Science 341.6141 (2013).
Implements Low-Error Amplicon Sequencing (LEA-Seq) method, described in: Faith, Jeremiah J., et al. "The long-term stability of the human gut microbiota." Science 341.6141 (2013).
@ghost
Copy link

ghost commented Apr 21, 2014

Build results will soon be (or already are) available at: http://ci.qiime.org/job/qiime-github-pr/824/

@cleme
Copy link
Contributor

cleme commented Apr 21, 2014

@ElBrogrammer just so you are aware this is in, take a quick look at it if you have some time this week

#!/usr/bin/env python
from __future__ import division

__author__ = "Jai Ram Rideout"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can change the author to yourself since you wrote most of this.

log_out: string
to be printed in log file
"""
log_out = "Quality filter results\nTotal number of input sequences: {}\nBarcode not in mapping file: {}\nSequence shorter than threshold: {}\nBarcode errors exceeds limit: {}\nPrimer mismatch count: {}\n\nTotal number seqs written: {}".format(input_seqs_count, barcode_not_in_map_count, seq_too_short_count, barcode_errors_exceed_max_count, primer_mismatch_count, total_seqs_kept)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trim down to 79 characters

@josenavas
Copy link
Member

Few more comments. @charudatta-navare I recomend to run flake8 on your files so no PEP8/FLAKE8 issues are found.

@cleme
Copy link
Contributor

cleme commented Oct 29, 2014

Thanks @josenavas! @charudatta-navare if you can take care of the issues pointed by Jose, we should be close to wrapping this up.

@charudatta-navare
Copy link
Contributor Author

@josenavas @cleme @jairideout
I have made the changes. Please let me know what should be the next steps.

@ghost
Copy link

ghost commented Oct 31, 2014

Build results will soon be (or already are) available at: http://ci.qiime.org/job/qiime-github-pr/1212/

@charudatta-navare
Copy link
Contributor Author

Looks like the checks are failing due to a different issue (pip can't proceed with requirement 'pynast==1.2.2 (from qiime==1.8.0-dev)' due to a pre-existing build directory.)

The tests are passing on my machine, so I think when the problem with pynast installation is solved, the tests should pass.

@cleme
Copy link
Contributor

cleme commented Oct 31, 2014

I am not exactly sure why you are getting that error, I think I read somewhere running pip with --force-reinstall on the install requirements fixes this, but not sure. @jairideout @gregcaporaso @josenavas any suggestions on this?

@charudatta-navare
Copy link
Contributor Author

The error comes in automated testing of the build on Jenkins. The tests fail after running for just over a minute.

@josenavas
Copy link
Member

Retest this please

@josenavas
Copy link
Member

Sometimes re-testing works

@ghost
Copy link

ghost commented Oct 31, 2014

Build results will soon be (or already are) available at: http://ci.qiime.org/job/qiime-github-pr/1214/

@jairideout
Copy link
Member

retest this please

@ghost
Copy link

ghost commented Oct 31, 2014

Build results will soon be (or already are) available at: http://ci.qiime.org/job/qiime-github-pr/1216/

@cleme cleme added this to the QIIME 1.9.0 milestone Nov 3, 2014
@josenavas
Copy link
Member

@charudatta-navare I don't see any issues with this. However, before merging, can you pull form upstream and push again? Now that #1700 has been merged, I'm not completely sure if all your skbio imports will work; so letting Jenkins to retest this with the latest qiime master will probably be safer. @cleme, if all the tests pass, I'm happy with merging these.

@ghost
Copy link

ghost commented Nov 12, 2014

Build results will soon be (or already are) available at: http://ci.qiime.org/job/qiime-github-pr/1241/

@ghost
Copy link

ghost commented Nov 12, 2014

Build results will soon be (or already are) available at: http://ci.qiime.org/job/qiime-github-pr/1242/

@charudatta-navare
Copy link
Contributor Author

Thanks for pointing that out, @josenavas! It needed a few changes in skbio imports, but that tests are passing now.
@cleme

@josenavas
Copy link
Member

Great! Thanks for the hard work @charudatta-navare
@cleme 👍 to merge

cleme added a commit that referenced this pull request Nov 12, 2014
@cleme cleme merged commit eea6ed8 into biocore:master Nov 12, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants