From 3a1a0a64f89bd61d2fb58c8cf9ebc9c6441cbba7 Mon Sep 17 00:00:00 2001 From: Dak Rojnuckarin Date: Fri, 7 Jul 2017 22:57:38 -0400 Subject: [PATCH 1/3] Attempt to fix issue #659 - Error parsing FASTA in multiple passes --- .../nbio/core/sequence/io/FastaReader.java | 39 ++--- .../nbio/core/fasta/TestFASTAReader.java | 150 ++++++++++++++++++ .../org/biojava/nbio/core/fasta/P02768.fasta | 12 ++ .../nbio/core/fasta/P02768_blank_lines.fasta | 21 +++ .../nbio/core/fasta/TwoSequences.fasta | 16 ++ .../core/fasta/TwoSequences_blank_lines.fasta | 21 +++ 6 files changed, 237 insertions(+), 22 deletions(-) create mode 100644 biojava-core/src/test/java/org/biojava/nbio/core/fasta/TestFASTAReader.java create mode 100644 biojava-core/src/test/resources/org/biojava/nbio/core/fasta/P02768.fasta create mode 100644 biojava-core/src/test/resources/org/biojava/nbio/core/fasta/P02768_blank_lines.fasta create mode 100644 biojava-core/src/test/resources/org/biojava/nbio/core/fasta/TwoSequences.fasta create mode 100644 biojava-core/src/test/resources/org/biojava/nbio/core/fasta/TwoSequences_blank_lines.fasta diff --git a/biojava-core/src/main/java/org/biojava/nbio/core/sequence/io/FastaReader.java b/biojava-core/src/main/java/org/biojava/nbio/core/sequence/io/FastaReader.java index 09eea5716c..8fddae0693 100644 --- a/biojava-core/src/main/java/org/biojava/nbio/core/sequence/io/FastaReader.java +++ b/biojava-core/src/main/java/org/biojava/nbio/core/sequence/io/FastaReader.java @@ -122,6 +122,8 @@ public LinkedHashMap process() throws IOException { * * @see #process() * @author Amr AL-Hossary @@ -188,37 +190,30 @@ public LinkedHashMap process(int max) throws IOException { line = br.readLine(); if (line == null) { - - - // Fix for #282 - if ( sequences.size() == 0 && max != -1) { - return null; - } - //i.e. EOF - String seq = sb.toString(); - if ( seq.length() == 0) { + if ( sb.length() == 0 && header.length() != 0 ) { logger.warn("Can't parse sequence {}. Got sequence of length 0!", sequenceIndex); logger.warn("header: {}", header); - } - //logger.info("Sequence index=" + sequenceIndex + " " + fileIndex ); - try { - @SuppressWarnings("unchecked") - S sequence = (S)sequenceCreator.getSequence(seq, sequenceIndex); - headerParser.parseHeader(header, sequence); - sequences.put(sequence.getAccession().getID(),sequence); - processedSequences++; - } catch (CompoundNotFoundException e) { - logger.warn("Sequence with header '{}' has unrecognised compounds ({}), it will be ignored", - header, e.getMessage()); + header = null; + } else if ( sb.length() > 0 ) { + //logger.info("Sequence index=" + sequenceIndex + " " + fileIndex ); + try { + @SuppressWarnings("unchecked") + S sequence = (S)sequenceCreator.getSequence(sb.toString(), sequenceIndex); + headerParser.parseHeader(header, sequence); + sequences.put(sequence.getAccession().getID(),sequence); + processedSequences++; + header = null; + } catch (CompoundNotFoundException e) { + logger.warn("Sequence with header '{}' has unrecognised compounds ({}), it will be ignored", + header, e.getMessage()); + } } keepGoing = false; } if (max > -1 && processedSequences>=max) { keepGoing=false; } - if ( this.line == null) - keepGoing = false; } while (keepGoing); this.line = line; diff --git a/biojava-core/src/test/java/org/biojava/nbio/core/fasta/TestFASTAReader.java b/biojava-core/src/test/java/org/biojava/nbio/core/fasta/TestFASTAReader.java new file mode 100644 index 0000000000..e4a9653cf5 --- /dev/null +++ b/biojava-core/src/test/java/org/biojava/nbio/core/fasta/TestFASTAReader.java @@ -0,0 +1,150 @@ +package org.biojava.nbio.core.fasta; + +import java.io.InputStream; +import java.util.LinkedHashMap; + +import static org.junit.Assert.* ; +import static org.hamcrest.CoreMatchers.* ; + +import org.biojava.nbio.core.sequence.ProteinSequence; +import org.biojava.nbio.core.sequence.compound.AminoAcidCompound; +import org.biojava.nbio.core.sequence.compound.AminoAcidCompoundSet; +import org.biojava.nbio.core.sequence.io.FastaReader; +import org.biojava.nbio.core.sequence.io.GenericFastaHeaderParser; +import org.biojava.nbio.core.sequence.io.ProteinSequenceCreator; +import org.biojava.nbio.core.sequence.io.util.ClasspathResource; +import org.junit.Test; + + +public class TestFASTAReader { + + private void testProcessAll(String path) throws Exception { + ClasspathResource r = new ClasspathResource(path); + FastaReader fastaReader = null ; + try( InputStream inStream = r.getInputStream() ) { + fastaReader = new FastaReader( + inStream, + new GenericFastaHeaderParser(), + new ProteinSequenceCreator(AminoAcidCompoundSet.getAminoAcidCompoundSet())); + LinkedHashMap sequences = fastaReader.process(); + assertThat(sequences,is(notNullValue())); + assertThat(sequences.size(),is(1)); + assertThat(sequences.containsKey("P02768"),is(true)); + assertThat(sequences.get("P02768").getLength(),is(609)); + } finally { + if(fastaReader != null) fastaReader.close(); + } + } + + /** + * Test file contains one sequence (P02768 from swissprot). Read the whole + * file all at once by calling {@link FastaReader#process()} and verify that + * one sequence is read. + * + * @throws Exception + */ + @Test + public void testProcessAll() throws Exception { + testProcessAll("org/biojava/nbio/core/fasta/P02768.fasta"); + } + + /** + * Same as {@link #testProcessAll()} but input files contains blank lines + * + * @throws Exception + */ + @Test + public void testProcessAllWithBlankLines() throws Exception { + testProcessAll("org/biojava/nbio/core/fasta/P02768_blank_lines.fasta"); + } + + private void testProcess1(String path) throws Exception { + ClasspathResource r = new ClasspathResource(path); + FastaReader fastaReader = null ; + try( InputStream inStream = r.getInputStream() ) { + fastaReader = new FastaReader( + inStream, + new GenericFastaHeaderParser(), + new ProteinSequenceCreator(AminoAcidCompoundSet.getAminoAcidCompoundSet())); + LinkedHashMap out1 = fastaReader.process(1); + assertThat(out1,is(notNullValue())); + assertThat(out1.size(),is(1)); + assertThat(out1.containsKey("P02768"),is(true)); + assertThat(out1.get("P02768").getLength(),is(609)); + LinkedHashMap out2 = fastaReader.process(1); + assertThat(out2,is(notNullValue())); + assertThat(out2.size(),is(0)); + } finally { + if(fastaReader != null) fastaReader.close(); + } + } + + /** + * Test file contains one sequence (P02768 from swissprot). Read one + * sequence at a time by calling {@link FastaReader#process(int)} and verify + * that the first call get one sequence and the second call get none. + * + * @throws Exception + */ + @Test + public void testProcess1() throws Exception { + testProcess1("org/biojava/nbio/core/fasta/P02768.fasta"); + } + + /** + * Same as {@link #testProcess1()}, but input contains blank lines. + * + * @throws Exception + */ + @Test + public void testProcess1WithBlankLines() throws Exception { + testProcess1("org/biojava/nbio/core/fasta/P02768_blank_lines.fasta"); + } + + private void testProcess2(String path) throws Exception { + ClasspathResource r = new ClasspathResource(path); + FastaReader fastaReader = null ; + try( InputStream inStream = r.getInputStream() ) { + fastaReader = new FastaReader( + inStream, + new GenericFastaHeaderParser(), + new ProteinSequenceCreator(AminoAcidCompoundSet.getAminoAcidCompoundSet())); + LinkedHashMap out1 = fastaReader.process(1); + assertThat(out1,is(notNullValue())); + assertThat(out1.size(),is(1)); + assertThat(out1.containsKey("P02768"),is(true)); + assertThat(out1.get("P02768").getLength(),is(609)); + LinkedHashMap out2 = fastaReader.process(1); + assertThat(out2,is(notNullValue())); + assertThat(out2.size(),is(1)); + assertThat(out2.containsKey("P00698"),is(true)); + assertThat(out2.get("P00698").getLength(),is(147)); + LinkedHashMap out3 = fastaReader.process(1); + assertThat(out3,is(notNullValue())); + assertThat(out3.size(),is(0)); + } finally { + if(fastaReader != null) fastaReader.close(); + } + } + + /** + * Test file contains two sequences. Read one sequence at a time by calling + * {@link FastaReader#process(int)} and verify that the first and second + * call get one sequence each and the third call get none. + * + * @throws Exception + */ + @Test + public void testProcess2() throws Exception { + testProcess2("org/biojava/nbio/core/fasta/TwoSequences.fasta"); + } + + /** + * Sane as {@link #testProcess2()} but input file contain blank lines + * @throws Exception + */ + @Test + public void testProcess2WithBlankLines() throws Exception { + testProcess2("org/biojava/nbio/core/fasta/TwoSequences_blank_lines.fasta"); + } +} diff --git a/biojava-core/src/test/resources/org/biojava/nbio/core/fasta/P02768.fasta b/biojava-core/src/test/resources/org/biojava/nbio/core/fasta/P02768.fasta new file mode 100644 index 0000000000..a435ae5a76 --- /dev/null +++ b/biojava-core/src/test/resources/org/biojava/nbio/core/fasta/P02768.fasta @@ -0,0 +1,12 @@ +>sp|P02768|ALBU_HUMAN Serum albumin OS=Homo sapiens GN=ALB PE=1 SV=2 +MKWVTFISLLFLFSSAYSRGVFRRDAHKSEVAHRFKDLGEENFKALVLIAFAQYLQQCPF +EDHVKLVNEVTEFAKTCVADESAENCDKSLHTLFGDKLCTVATLRETYGEMADCCAKQEP +ERNECFLQHKDDNPNLPRLVRPEVDVMCTAFHDNEETFLKKYLYEIARRHPYFYAPELLF +FAKRYKAAFTECCQAADKAACLLPKLDELRDEGKASSAKQRLKCASLQKFGERAFKAWAV +ARLSQRFPKAEFAEVSKLVTDLTKVHTECCHGDLLECADDRADLAKYICENQDSISSKLK +ECCEKPLLEKSHCIAEVENDEMPADLPSLAADFVESKDVCKNYAEAKDVFLGMFLYEYAR +RHPDYSVVLLLRLAKTYETTLEKCCAAADPHECYAKVFDEFKPLVEEPQNLIKQNCELFE +QLGEYKFQNALLVRYTKKVPQVSTPTLVEVSRNLGKVGSKCCKHPEAKRMPCAEDYLSVV +LNQLCVLHEKTPVSDRVTKCCTESLVNRRPCFSALEVDETYVPKEFNAETFTFHADICTL +SEKERQIKKQTALVELVKHKPKATKEQLKAVMDDFAAFVEKCCKADDKETCFAEEGKKLV +AASQAALGL \ No newline at end of file diff --git a/biojava-core/src/test/resources/org/biojava/nbio/core/fasta/P02768_blank_lines.fasta b/biojava-core/src/test/resources/org/biojava/nbio/core/fasta/P02768_blank_lines.fasta new file mode 100644 index 0000000000..2d56071f34 --- /dev/null +++ b/biojava-core/src/test/resources/org/biojava/nbio/core/fasta/P02768_blank_lines.fasta @@ -0,0 +1,21 @@ +>sp|P02768|ALBU_HUMAN Serum albumin OS=Homo sapiens GN=ALB PE=1 SV=2 +MKWVTFISLLFLFSSAYSRGVFRRDAHKSEVAHRFKDLGEENFKALVLIAFAQYLQQCPF +EDHVKLVNEVTEFAKTCVADESAENCDKSLHTLFGDKLCTVATLRETYGEMADCCAKQEP +ERNECFLQHKDDNPNLPRLVRPEVDVMCTAFHDNEETFLKKYLYEIARRHPYFYAPELLF +FAKRYKAAFTECCQAADKAACLLPKLDELRDEGKASSAKQRLKCASLQKFGERAFKAWAV + + + +ARLSQRFPKAEFAEVSKLVTDLTKVHTECCHGDLLECADDRADLAKYICENQDSISSKLK +ECCEKPLLEKSHCIAEVENDEMPADLPSLAADFVESKDVCKNYAEAKDVFLGMFLYEYAR +RHPDYSVVLLLRLAKTYETTLEKCCAAADPHECYAKVFDEFKPLVEEPQNLIKQNCELFE +QLGEYKFQNALLVRYTKKVPQVSTPTLVEVSRNLGKVGSKCCKHPEAKRMPCAEDYLSVV +LNQLCVLHEKTPVSDRVTKCCTESLVNRRPCFSALEVDETYVPKEFNAETFTFHADICTL +SEKERQIKKQTALVELVKHKPKATKEQLKAVMDDFAAFVEKCCKADDKETCFAEEGKKLV +AASQAALGL + + + + + + diff --git a/biojava-core/src/test/resources/org/biojava/nbio/core/fasta/TwoSequences.fasta b/biojava-core/src/test/resources/org/biojava/nbio/core/fasta/TwoSequences.fasta new file mode 100644 index 0000000000..2fde8a3f43 --- /dev/null +++ b/biojava-core/src/test/resources/org/biojava/nbio/core/fasta/TwoSequences.fasta @@ -0,0 +1,16 @@ +>sp|P02768|ALBU_HUMAN Serum albumin OS=Homo sapiens GN=ALB PE=1 SV=2 +MKWVTFISLLFLFSSAYSRGVFRRDAHKSEVAHRFKDLGEENFKALVLIAFAQYLQQCPF +EDHVKLVNEVTEFAKTCVADESAENCDKSLHTLFGDKLCTVATLRETYGEMADCCAKQEP +ERNECFLQHKDDNPNLPRLVRPEVDVMCTAFHDNEETFLKKYLYEIARRHPYFYAPELLF +FAKRYKAAFTECCQAADKAACLLPKLDELRDEGKASSAKQRLKCASLQKFGERAFKAWAV +ARLSQRFPKAEFAEVSKLVTDLTKVHTECCHGDLLECADDRADLAKYICENQDSISSKLK +ECCEKPLLEKSHCIAEVENDEMPADLPSLAADFVESKDVCKNYAEAKDVFLGMFLYEYAR +RHPDYSVVLLLRLAKTYETTLEKCCAAADPHECYAKVFDEFKPLVEEPQNLIKQNCELFE +QLGEYKFQNALLVRYTKKVPQVSTPTLVEVSRNLGKVGSKCCKHPEAKRMPCAEDYLSVV +LNQLCVLHEKTPVSDRVTKCCTESLVNRRPCFSALEVDETYVPKEFNAETFTFHADICTL +SEKERQIKKQTALVELVKHKPKATKEQLKAVMDDFAAFVEKCCKADDKETCFAEEGKKLV +AASQAALGL +>sp|P00698|LYSC_CHICK Lysozyme C OS=Gallus gallus GN=LYZ PE=1 SV=1 +MRSLLILVLCFLPLAALGKVFGRCELAAAMKRHGLDNYRGYSLGNWVCAAKFESNFNTQA +TNRNTDGSTDYGILQINSRWWCNDGRTPGSRNLCNIPCSALLSSDITASVNCAKKIVSDG +NGMNAWVAWRNRCKGTDVQAWIRGCRL \ No newline at end of file diff --git a/biojava-core/src/test/resources/org/biojava/nbio/core/fasta/TwoSequences_blank_lines.fasta b/biojava-core/src/test/resources/org/biojava/nbio/core/fasta/TwoSequences_blank_lines.fasta new file mode 100644 index 0000000000..78006dd898 --- /dev/null +++ b/biojava-core/src/test/resources/org/biojava/nbio/core/fasta/TwoSequences_blank_lines.fasta @@ -0,0 +1,21 @@ +>sp|P02768|ALBU_HUMAN Serum albumin OS=Homo sapiens GN=ALB PE=1 SV=2 +MKWVTFISLLFLFSSAYSRGVFRRDAHKSEVAHRFKDLGEENFKALVLIAFAQYLQQCPF +EDHVKLVNEVTEFAKTCVADESAENCDKSLHTLFGDKLCTVATLRETYGEMADCCAKQEP +ERNECFLQHKDDNPNLPRLVRPEVDVMCTAFHDNEETFLKKYLYEIARRHPYFYAPELLF + + +FAKRYKAAFTECCQAADKAACLLPKLDELRDEGKASSAKQRLKCASLQKFGERAFKAWAV +ARLSQRFPKAEFAEVSKLVTDLTKVHTECCHGDLLECADDRADLAKYICENQDSISSKLK +ECCEKPLLEKSHCIAEVENDEMPADLPSLAADFVESKDVCKNYAEAKDVFLGMFLYEYAR +RHPDYSVVLLLRLAKTYETTLEKCCAAADPHECYAKVFDEFKPLVEEPQNLIKQNCELFE +QLGEYKFQNALLVRYTKKVPQVSTPTLVEVSRNLGKVGSKCCKHPEAKRMPCAEDYLSVV +LNQLCVLHEKTPVSDRVTKCCTESLVNRRPCFSALEVDETYVPKEFNAETFTFHADICTL +SEKERQIKKQTALVELVKHKPKATKEQLKAVMDDFAAFVEKCCKADDKETCFAEEGKKLV +AASQAALGL + + +>sp|P00698|LYSC_CHICK Lysozyme C OS=Gallus gallus GN=LYZ PE=1 SV=1 +MRSLLILVLCFLPLAALGKVFGRCELAAAMKRHGLDNYRGYSLGNWVCAAKFESNFNTQA + +TNRNTDGSTDYGILQINSRWWCNDGRTPGSRNLCNIPCSALLSSDITASVNCAKKIVSDG +NGMNAWVAWRNRCKGTDVQAWIRGCRL \ No newline at end of file From 4f301ce15c3149cc5e85f261ca081e8ee16d0408 Mon Sep 17 00:00:00 2001 From: Dak Rojnuckarin Date: Sat, 8 Jul 2017 00:01:51 -0400 Subject: [PATCH 2/3] Revert to old behavior where process(int) return null on end-of-file. --- .../java/org/biojava/nbio/core/sequence/io/FastaReader.java | 4 +--- .../java/org/biojava/nbio/core/fasta/TestFASTAReader.java | 6 ++---- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/biojava-core/src/main/java/org/biojava/nbio/core/sequence/io/FastaReader.java b/biojava-core/src/main/java/org/biojava/nbio/core/sequence/io/FastaReader.java index 8fddae0693..4ad7dbeb9c 100644 --- a/biojava-core/src/main/java/org/biojava/nbio/core/sequence/io/FastaReader.java +++ b/biojava-core/src/main/java/org/biojava/nbio/core/sequence/io/FastaReader.java @@ -122,8 +122,6 @@ public LinkedHashMap process() throws IOException { *
    *
  • This method can't be called after calling its NO-ARGUMENT twin.
    • *
  • remember to close the underlying resource when you are done.
    • - *
  • This method always return non-null hash map. When end-of-file is reached, - * the number of sequences returned will be smaller than the number requested.
    • *
* @see #process() * @author Amr AL-Hossary @@ -219,7 +217,7 @@ public LinkedHashMap process(int max) throws IOException { this.line = line; this.header= header; - return sequences; + return max > -1 && sequences.isEmpty() ? null : sequences; } public void close() throws IOException { diff --git a/biojava-core/src/test/java/org/biojava/nbio/core/fasta/TestFASTAReader.java b/biojava-core/src/test/java/org/biojava/nbio/core/fasta/TestFASTAReader.java index e4a9653cf5..2458fd5f1f 100644 --- a/biojava-core/src/test/java/org/biojava/nbio/core/fasta/TestFASTAReader.java +++ b/biojava-core/src/test/java/org/biojava/nbio/core/fasta/TestFASTAReader.java @@ -72,8 +72,7 @@ private void testProcess1(String path) throws Exception { assertThat(out1.containsKey("P02768"),is(true)); assertThat(out1.get("P02768").getLength(),is(609)); LinkedHashMap out2 = fastaReader.process(1); - assertThat(out2,is(notNullValue())); - assertThat(out2.size(),is(0)); + assertThat(out2,is(nullValue())); } finally { if(fastaReader != null) fastaReader.close(); } @@ -120,8 +119,7 @@ private void testProcess2(String path) throws Exception { assertThat(out2.containsKey("P00698"),is(true)); assertThat(out2.get("P00698").getLength(),is(147)); LinkedHashMap out3 = fastaReader.process(1); - assertThat(out3,is(notNullValue())); - assertThat(out3.size(),is(0)); + assertThat(out3,is(nullValue())); } finally { if(fastaReader != null) fastaReader.close(); } From 76e2499a71209589669a253f84fee438920f518e Mon Sep 17 00:00:00 2001 From: Dak Rojnuckarin Date: Sat, 8 Jul 2017 14:24:34 -0400 Subject: [PATCH 3/3] Last travis-CI build got cancelled. Re-push to trigger it again. --- .../org/biojava/nbio/core/fasta/P02768_blank_lines.fasta | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/biojava-core/src/test/resources/org/biojava/nbio/core/fasta/P02768_blank_lines.fasta b/biojava-core/src/test/resources/org/biojava/nbio/core/fasta/P02768_blank_lines.fasta index 2d56071f34..6df955ab62 100644 --- a/biojava-core/src/test/resources/org/biojava/nbio/core/fasta/P02768_blank_lines.fasta +++ b/biojava-core/src/test/resources/org/biojava/nbio/core/fasta/P02768_blank_lines.fasta @@ -4,7 +4,7 @@ EDHVKLVNEVTEFAKTCVADESAENCDKSLHTLFGDKLCTVATLRETYGEMADCCAKQEP ERNECFLQHKDDNPNLPRLVRPEVDVMCTAFHDNEETFLKKYLYEIARRHPYFYAPELLF FAKRYKAAFTECCQAADKAACLLPKLDELRDEGKASSAKQRLKCASLQKFGERAFKAWAV - + ARLSQRFPKAEFAEVSKLVTDLTKVHTECCHGDLLECADDRADLAKYICENQDSISSKLK ECCEKPLLEKSHCIAEVENDEMPADLPSLAADFVESKDVCKNYAEAKDVFLGMFLYEYAR