Resolve more PR change requests

Author: johnpalsberg

flatgfa/src/flatgfa.rs

@@ -4,7 +4,7 @@ use std::ops::Range;
 use std::str::FromStr;
 
 use crate::{
-    packedseq::{compress_into_buffer, PackedSeqView, SeqSpan},
+    packedseq::{PackedSeqView, SeqSpan},
     pool::{self, Id, Pool, Span, Store},
 };
 use bstr::BStr;
@@ -435,13 +435,44 @@ impl<'a, P: StoreFamily<'a>> GFAStore<'a, P> {
         self.header.add_slice(version);
     }
 
-    /// Add a new segment to the GFA file.
-    pub fn add_seg(&mut self, name: usize, seq: &[u8], optional: &[u8]) -> Id<Segment> {
-        let mut compressed: Vec<u8> = Vec::new();
-        let end_offset = compress_into_buffer(seq, &mut compressed);
-        let byte_span = self.seq_data.add_slice(&compressed);
+    /// Add a new segment to the GFA file, compressing the data in `seq`
+    pub fn compress_and_add_seg(
+        &mut self,
+        name: usize,
+        seq: &[u8],
+        optional: &[u8],
+    ) -> Id<Segment> {
+        self.seq_data.reserve(seq.len());
+        let mut high_nibble_end = true;
+        let mut combined_item = 0;
+        let start_id = self.seq_data.next_id();
+        for i in 0..seq.len() {
+            let item = seq[i];
+            let converted: u8 = match item {
+                65 => 0,
+                67 => 1,
+                84 => 2,
+                71 => 3,
+                78 => 4,
+                _ => panic!("Not a Nucleotide!"),
+            };
+            if high_nibble_end {
+                if i == seq.len() - 1 {
+                    self.seq_data.add(converted);
+                    break;
+                }
+                combined_item = converted;
+                high_nibble_end = false;
+            } else {
+                combined_item |= converted << 4;
+                self.seq_data.add(combined_item);
+                high_nibble_end = true;
+            }
+        }
+        let end_id = self.seq_data.next_id();
+        let byte_span = Span::new(start_id, end_id);
         let start = SeqSpan::to_logical(byte_span.start.index(), false);
-        let end = SeqSpan::to_logical(byte_span.end.index() - 1, end_offset) + 1;
+        let end = SeqSpan::to_logical(byte_span.end.index() - 1, high_nibble_end) + 1;
         self.segs.add(Segment {
             name,
             seq: SeqSpan {
@@ -453,21 +484,13 @@ impl<'a, P: StoreFamily<'a>> GFAStore<'a, P> {
     }
 
     /// Add a new segment with already compressed data
-    pub fn add_seg_already_compressed(
-        &mut self,
-        name: usize,
-        seq: PackedSeqView,
-        optional: &[u8],
-    ) -> Id<Segment> {
+    pub fn add_seg(&mut self, name: usize, seq: PackedSeqView, optional: &[u8]) -> Id<Segment> {
         let byte_span = self.seq_data.add_slice(seq.data);
         let start = SeqSpan::to_logical(byte_span.start.index(), seq.high_nibble_begin);
         let end = SeqSpan::to_logical(byte_span.end.index() - 1, seq.high_nibble_end) + 1;
         self.segs.add(Segment {
             name,
-            seq: SeqSpan {
-                start,
-                len: (end - start) as u16,
-            },
+            seq: (start as usize..end as usize).into(),
             optional: self.optional_data.add_slice(optional),
         })
     }

flatgfa/src/ops/chop.rs

@@ -1,3 +1,5 @@
+use std::ops::Range;
+
 use crate::flatgfa::{self, Handle, Link, Orientation, Path, Segment};
 use crate::packedseq::SeqSpan;
 use crate::pool::{Id, Span, Store};
@@ -35,7 +37,7 @@ pub fn chop(gfa: &flatgfa::FlatGFA, max_size: usize, incl_links: bool) -> flatgf
             max_node_id += 1;
             seg_map.push(Span::new(id, flat.segs.next_id()));
         } else {
-            let seq_range = seg.seq.to_range();
+            let seq_range: Range<usize> = seg.seq.into();
             let seq_end = seq_range.end;
             let mut offset = seq_range.start;
             let segs_start = flat.segs.next_id();
@@ -46,11 +48,11 @@ pub fn chop(gfa: &flatgfa::FlatGFA, max_size: usize, incl_links: bool) -> flatgf
                 // Generate a new segment of length c
                 flat.segs.add(Segment {
                     name: max_node_id,
-                    seq: SeqSpan::from_range(std::ops::Range {
-                        // Note for reviwer: Change made here
+                    seq: std::ops::Range {
                         start: offset,
                         end: offset + max_size,
-                    }),
+                    }
+                    .into(),
                     optional: Span::new_empty(),
                 });
                 offset += max_size;
@@ -59,11 +61,11 @@ pub fn chop(gfa: &flatgfa::FlatGFA, max_size: usize, incl_links: bool) -> flatgf
             // Generate the last segment
             flat.segs.add(Segment {
                 name: max_node_id,
-                seq: SeqSpan::from_range(std::ops::Range {
-                    // Note for reviwer: Change made here
+                seq: std::ops::Range {
                     start: offset,
                     end: seq_end,
-                }),
+                }
+                .into(),
                 optional: Span::new_empty(),
             });
             max_node_id += 1;

flatgfa/src/ops/extract.rs

@@ -36,7 +36,7 @@ impl<'a> SubgraphBuilder<'a> {
     /// Add a segment from the source graph to this subgraph.
     fn include_seg(&mut self, seg_id: Id<Segment>) {
         let seg = &self.old.segs[seg_id];
-        let new_seg_id = self.store.add_seg_already_compressed(
+        let new_seg_id = self.store.add_seg(
             // Note for reviwer, change made here
             seg.name,
             self.old.get_seq(seg),

flatgfa/src/packedseq.rs

@@ -284,7 +284,7 @@ impl<'a> PackedSeqView<'a> {
     /// Given a pool of compressed data (`pool`), create a PackedSeqView in the range of `span`
     ///
     pub fn from_pool(pool: Pool<'a, u8>, span: SeqSpan) -> Self {
-        let slice = &pool.all()[span.start_byte_index()..span.end_byte_index()];
+        let slice = &pool.all()[span.byte_range()];
         Self {
             data: slice,
             high_nibble_begin: span.get_nibble_begin(),
@@ -443,37 +443,12 @@ impl SeqSpan {
         self.len == 0
     }
 
-    /// Given `range`, returns the equivalent SeqSpan
-    pub fn from_range(range: Range<usize>) -> Self {
-        Self {
-            start: range.start as u32,
-            len: (range.end - range.start) as u16,
-        }
-    }
-
-    /// Returns the range that is equivalent to this SeqSpan
-    pub fn to_range(&self) -> Range<usize> {
-        Range {
-            start: self.start as usize,
-            end: self.end(),
-        }
-    }
-
     // Returns the logical index of the element given the byte index and nibble offset
     pub fn to_logical(byte_index: usize, end_offset: bool) -> u32 {
         (byte_index * 2 + end_offset as usize) as u32
     }
 
-    // Returns the index of the starting byte
-    pub fn start_byte_index(&self) -> usize {
-        (self.start / 2) as usize
-    }
-
-    // Returns the index one greater than the end byte index
-    pub fn end_byte_index(&self) -> usize {
-        self.end().div_ceil(2)
-    }
-
+    // Returns a range of the bytes covered by this SeqSpan
     pub fn byte_range(&self) -> Range<usize> {
         Range {
             start: (self.start / 2) as usize,
@@ -549,30 +524,6 @@ pub fn export(seq: PackedSeqView, filename: &str) {
     seq.write_file(&mut mem);
 }
 
-/// Takes a slice of uncompressed ASCII-encoded base pairs, compresses them and pushes them into `output`
-pub fn compress_into_buffer(input: &[u8], output: &mut Vec<u8>) -> bool {
-    let mut high_nibble_end = true;
-    for item in input {
-        let converted: u8 = match item {
-            65 => 0,
-            67 => 1,
-            84 => 2,
-            71 => 3,
-            78 => 4,
-            _ => panic!("Not a Nucleotide!"),
-        };
-        if high_nibble_end {
-            output.push(converted);
-            high_nibble_end = false;
-        } else {
-            let last_index = output.len() - 1;
-            output[last_index] |= converted << 4;
-            high_nibble_end = true;
-        }
-    }
-    high_nibble_end
-}
-
 /// Takes a slice of compressed base pairs, decompresses them and pushes them into `output`
 pub fn decompress_into_buffer(input: PackedSeqView, output: &mut Vec<u8>) {
     if !input.high_nibble_begin {

flatgfa/src/parse.rs

@@ -136,7 +136,7 @@ impl<'a, P: flatgfa::StoreFamily<'a>> Parser<'a, P> {
     }
 
     fn add_seg(&mut self, seg: gfaline::Segment) {
-        let seg_id = self.flat.add_seg(seg.name, seg.seq, seg.data);
+        let seg_id = self.flat.compress_and_add_seg(seg.name, seg.seq, seg.data);
         self.seg_ids.insert(seg.name, seg_id);
     }
 

flatgfa/src/pool.rs

@@ -147,6 +147,9 @@ pub trait Store<T: Clone> {
     fn next_id(&self) -> Id<T> {
         Id::new(self.len())
     }
+
+    /// Reserve space for directly adding elements
+    fn reserve(&mut self, num_elems: usize);
 }
 
 /// A store that uses a `Vec` to allocate objects on the heap.
@@ -181,6 +184,10 @@ impl<T: Clone> Store<T> for HeapStore<T> {
     fn len(&self) -> usize {
         self.0.len()
     }
+
+    fn reserve(&mut self, num_elems: usize) {
+        self.0.reserve(num_elems);
+    }
 }
 
 impl<T> Default for HeapStore<T> {
@@ -223,6 +230,11 @@ impl<T: Clone> Store<T> for FixedStore<'_, T> {
     fn len(&self) -> usize {
         self.0.len()
     }
+
+    fn reserve(&mut self, num_elems: usize) {
+        let required_capacity = self.len() + num_elems;
+        assert!(self.capacity() >= required_capacity);
+    }
 }
 
 impl<T> FixedStore<'_, T> {