cleaned up code

Author: Johanna

src/cli.rs

@@ -207,13 +207,13 @@ pub enum Commands {
 
     /// Delete genome(s) from a database (input: one id per line)
     Delete {
-        /// Sketching database basename (so without .skm or .skd)
+        /// Sketching database
         #[arg(required = true)]
         db: String,
 
         /// Input file with IDs to delete (one ID per line)
         #[arg(required = true)]
-        genome_ids: String,
+        samples: String,
 
         /// output file name
         #[arg(required = true)]

src/lib.rs

@@ -486,13 +486,13 @@ pub fn main() -> Result<(), Error> {
 
         Commands::Delete {
             db,
-            genome_ids,
+            samples,
             output_file,
         } => {
             let ref_db = utils::strip_sketch_extension(db);
 
             log::info!("Reading input genomes");
-            let path = Path::new(genome_ids);
+            let path = Path::new(samples);
             let file = File::open(path)?;
             let reader = std::io::BufReader::new(file);
 
@@ -503,8 +503,9 @@ pub fn main() -> Result<(), Error> {
             let mut sketches: MultiSketch = MultiSketch::load(ref_db)
                 .unwrap_or_else(|_| panic!("Could not read sketch metadata from {}.skm", ref_db));
 
+            //TODO: check if all genome IDs have been found in meta and sketchdata
             // write new .skm
-            let _ = sketches.remove_metadata(output_file, &ids);
+            sketches.remove_metadata(output_file, &ids)?;
 
             // remove samples from .skd file
             log::info!("Remove genomes and writing output");

src/multisketch.rs

@@ -1,4 +1,7 @@
+use anyhow::bail;
 use anyhow::Error;
+use anyhow::{Result, anyhow};
+// use thiserror::Error;
 use core::panic;
 use std::fmt;
 use std::fs::File;
@@ -12,6 +15,7 @@ use crate::hashing::HashType;
 use crate::sketch::{Sketch, BBITS};
 use crate::sketch_datafile::SketchArrayFile;
 
+use std::collections::HashSet;
 #[derive(Serialize, Deserialize)]
 pub struct MultiSketch {
     pub sketch_size: u64,
@@ -195,17 +199,32 @@ impl MultiSketch {
         &mut self,
         output_file_name: &str,
         genome_ids_to_remove: &[String],
-    ) -> std::io::Result<()> {
-        println!("{}", self);
-        let mut new_sketch_metadata: Vec<Sketch> = Vec::with_capacity(self.sketch_metadata.len());
+    ) -> anyhow::Result<()> {
+        // error for write batch
+        // error for if IDs are missing
+        let mut new_sketch_metadata: Vec<Sketch> =
+            Vec::with_capacity(self.sketch_metadata.len() - genome_ids_to_remove.len());
+        let mut removed_samples = Vec::new();
 
         for sketch in &self.sketch_metadata {
+            
             if !genome_ids_to_remove.contains(&(*sketch.name()).to_string()) {
                 new_sketch_metadata.push(sketch.clone());
+
+            } else {
+                removed_samples.push(sketch.name());
             }
         }
+
+        let set1: HashSet<&str> = removed_samples.iter().map(AsRef::as_ref).collect();
+        let set2: HashSet<&str> = genome_ids_to_remove.iter().map(AsRef::as_ref).collect();
+        let missing: Vec<&&str> = set2.difference(&set1).collect();
+        if !missing.is_empty() {
+            bail!("Elements in set2 not found in set1: {:?}", missing);
+        }        
+
         self.sketch_metadata = new_sketch_metadata;
-        let _ = self.save_metadata(output_file_name);
+        self.save_metadata(output_file_name)?;
         Ok(())
     }
 
@@ -214,21 +233,20 @@ impl MultiSketch {
         input_prefix: &str,
         output_file: &str,
         genome_ids_to_remove: &[String],
-    ) -> std::io::Result<()> {
-        // Check if all genome IDs to remove exist and get their positions
+    ) -> anyhow::Result<()>  {
         let mut positions_to_remove = Vec::new();
         let mut missing_ids = Vec::new();
 
         for id in genome_ids_to_remove {
-            println!("{}", id);
             if let Some(&position) = self.name_map.get(id) {
                 positions_to_remove.push(position);
             } else {
                 missing_ids.push(id);
             }
         }
+
         if !missing_ids.is_empty() {
-            panic!("The following genome IDs were not found: {:?}", missing_ids);
+            bail!("The following genome IDs were not found: {:?}", missing_ids);
         }
 
         // Create a list of indices to keep
@@ -238,17 +256,14 @@ impl MultiSketch {
 
         let input_filename = format!("{}.skd", input_prefix);
         let output_filename = format!("{}.skd", output_file);
-        SketchArrayFile::write_batch(
+        if let Err(e) = SketchArrayFile::write_batch(
             &input_filename,
             &output_filename,
             &indices_to_keep,
             self.sample_stride,
-        )
-        .unwrap_or_else(|e| {
-            eprintln!("Error during batch write: {}", e);
-            std::process::exit(1);
-        });
-        println!("Output sketch data written to: {output_filename}",);
+        ) {
+            return Err(anyhow!("Error during batch write: {}", e).into());
+        }
 
         Ok(())
     }

src/sketch.rs

@@ -150,7 +150,7 @@ impl Sketch {
         }
     }
 
-    fn clean_name(n: &str) -> String {
+    fn remove_extension(n: &str) -> String {
         let stem = Path::new(n).file_stem().unwrap().to_str().unwrap();
         stem.strip_suffix(".fa")
             .or_else(|| stem.strip_suffix(".fasta"))
@@ -269,11 +269,10 @@ pub fn sketch_files(
                         .iter_mut()
                         .enumerate()
                         .map(|(idx, hash_it)| {
-                            // changed how the sample name is processed
                             let sample_name = if concat_fasta {
                                 format!("{name}_{}", idx + 1)
                             } else {
-                                Sketch::clean_name(name)
+                                Sketch::remove_extension(name)
                             };
                             if hash_it.seq_len() == 0 {
                                 panic!("{sample_name} has no valid sequence");

tests/delete.rs

@@ -4,15 +4,13 @@ use std::path::Path;
 
 pub mod common;
 use crate::common::*;
-use sketchlib::io::*;
 
 use sketchlib::multisketch::MultiSketch;
 
 #[cfg(test)]
 
 mod tests {
     use super::*;
-    use sketchlib::io;
 
     #[test]
     fn test_delete_sketches() {
@@ -78,3 +76,83 @@ mod tests {
         );
     }
 }
+
+// use predicates::prelude::*;
+// use snapbox::cmd::{cargo_bin, Command};
+// use std::path::Path;
+
+// pub mod common;
+// use crate::common::*;
+// use sketchlib::io::*;
+
+// use sketchlib::multisketch::MultiSketch;
+
+// #[cfg(test)]
+
+// mod tests {
+//     use super::*;
+//     use sketchlib::io;
+
+//     #[test]
+//     fn test_delete_sketches() {
+//         let sandbox = TestSetup::setup();
+
+//         Command::new(cargo_bin("sketchlib"))
+//             .current_dir(sandbox.get_wd())
+//             .arg("sketch")
+//             .args(&["--k-vals", "17"])
+//             .arg("--seq-files")
+//             .arg(sandbox.file_string("14412_3#82.contigs_velvet.fa.gz", TestDir::Input))
+//             .arg(sandbox.file_string("14412_3#84.contigs_velvet.fa.gz", TestDir::Input))
+//             .arg(sandbox.file_string("R6.fa.gz", TestDir::Input))
+//             .arg(sandbox.file_string("TIGR4.fa.gz", TestDir::Input))
+//             .arg("-v")
+//             .args(&["-o", "full_db"])
+//             .assert()
+//             .success();
+
+//         Command::new(cargo_bin("sketchlib"))
+//             .current_dir(sandbox.get_wd())
+//             .arg("sketch")
+//             .args(&["--k-vals", "17"])
+//             .arg("--seq-files")
+//             .arg(sandbox.file_string("14412_3#82.contigs_velvet.fa.gz", TestDir::Input))
+//             .arg(sandbox.file_string("14412_3#84.contigs_velvet.fa.gz", TestDir::Input))
+//             .arg("-v")
+//             .args(&["-o", "result_db"])
+//             .assert()
+//             .success();
+
+//         // samples in delete_test.txt are TIGR4, and R6
+//         Command::new(cargo_bin("sketchlib"))
+//             .current_dir(sandbox.get_wd())
+//             .arg("delete")
+//             .arg("full_db")
+//             .arg(sandbox.file_string("delete_test.txt", TestDir::Input))
+//             .arg("deleted_db")
+//             .assert()
+//             .success();
+
+//         let predicate_file = predicate::path::eq_file(Path::new(
+//             &sandbox.file_string("deleted_db.skd", TestDir::Output),
+//         ));
+//         assert_eq!(
+//             true,
+//             predicate_file.eval(Path::new(
+//                 &sandbox.file_string("result_db.skd", TestDir::Output)
+//             )),
+//             "Sketch data after deletion incorrect"
+//         );
+
+//         // Check .skm the same
+//         let merged_sketch: MultiSketch =
+//             MultiSketch::load(&sandbox.file_string("deleted_db", TestDir::Output))
+//                 .expect("Failed to load output merged sketch");
+//         let expected_sketch = MultiSketch::load(&sandbox.file_string("result_db", TestDir::Output))
+//             .expect("Failed to load expected merged sketch");
+//         assert_eq!(
+//             merged_sketch, expected_sketch,
+//             "Metadata data after deletion incorrect"
+//         );
+//     }
+// }