Repository: bacpop/ska.rust (rust)

Issue:
minor suggestions about "ska delete"
2 minor suggestions about the command ska delete (tried using ska v0.3.5):
     _ to add an output option to not necessarily overwrite the original skf file
     _ it seems that the original skf file should be specified first before the sample names while the help message indicates otherwise:
Usage: ska delete [OPTIONS] <NAMES|-f <FILE_LIST>> <SKF_FILE>


Pull request description:
ska delete upgrade
Closes #79 

Patch:
diff --git a/Cargo.toml b/Cargo.toml
index 6f420c8..a84a884 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "ska"
-version = "0.3.9"
+version = "0.3.10"
 authors = [
     "John Lees <jlees@ebi.ac.uk>",
     "Simon Harris <simon.harris@gatesfoundation.org>",
diff --git a/src/cli.rs b/src/cli.rs
index 8e32cb8..dcdf0c8 100644
--- a/src/cli.rs
+++ b/src/cli.rs
@@ -280,8 +280,13 @@ pub enum Commands {
     /// Remove samples from a split k-mer file
     Delete {
         /// Split-kmer (.skf) file to operate on
+        #[arg(short, long, required = true)]
         skf_file: String,
 
+        /// Output name. If not provided, will overwrite the input file
+        #[arg(short)]
+        output: Option<String>,
+
         /// File listing sample names to remove
         #[arg(short, group = "input")]
         file_list: Option<String>,
diff --git a/src/coverage.rs b/src/coverage.rs
index 0b0018d..2a8614f 100644
--- a/src/coverage.rs
+++ b/src/coverage.rs
@@ -148,7 +148,6 @@ where
     ///
     /// # Panics
     /// - If the fit has already been run
-    #[allow(clippy::map_clone)]
     pub fn fit_histogram(&mut self) -> Result<usize, Error> {
         if self.fitted {
             panic!("Model already fitted");
@@ -169,7 +168,7 @@ where
             .iter()
             .rev()
             .skip_while(|x| **x < MIN_FREQ)
-            .map(|x| *x)
+            .copied()
             .collect();
         self.counts.reverse();
         let counts_f64: Vec<f64> = self.counts.iter().map(|x| *x as f64).collect();
diff --git a/src/generic_modes.rs b/src/generic_modes.rs
index b49f46a..9b53be8 100644
--- a/src/generic_modes.rs
+++ b/src/generic_modes.rs
@@ -209,9 +209,15 @@ pub fn delete<IntT: for<'a> UInt<'a>>(
     log::info!("Deleting samples");
     ska_array.delete_samples(delete_names);
 
-    log::info!("Saving modified skf file");
+    // Conditionally add suffix
+    let outfile_suffix = if out_file.ends_with(".skf") {
+        out_file.to_string()
+    } else {
+        format!("{out_file}.skf")
+    };
+    log::info!("Saving modified skf file to {outfile_suffix}");
     ska_array
-        .save(out_file)
+        .save(&outfile_suffix)
         .expect("Could not save modified array");
 }
 
diff --git a/src/lib.rs b/src/lib.rs
index 1eeb3a2..6601751 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -236,11 +236,12 @@
 //! ## ska delete
 //!
 //! Remove samples by name from an `.skf` file. All sample names must exist in the
-//! file, or an error will be thrown. After removing samples, the input `.skf` file will be overwritten.
+//! file, or an error will be thrown. After removing samples, the input `.skf` file will be overwritten,
+//! unless you provide a new one with `-o`.
 //! Any split k-mers which have no observed missing bases after removal will also be deleted.
 //!
 //! ```bash
-//! ska delete all_samples.skf sample_1 sample_3
+//! ska delete --skf-file all_samples.skf sample_1 sample_3
 //! ```
 //! If you wish to delete many samples, you can use `-f` to provide their names
 //! by line via an input file.
@@ -662,16 +663,18 @@ pub fn main() {
         }
         Commands::Delete {
             skf_file,
+            output,
             file_list,
             names,
         } => {
             let input_files = get_input_list(file_list, names);
             let input_names: Vec<&str> = input_files.iter().map(|t| &*t.0).collect();
+            let output_file = output.clone().unwrap_or(skf_file.to_string());
             log::info!("Loading skf file");
             if let Ok(mut ska_array) = MergeSkaArray::<u64>::load(skf_file) {
-                delete(&mut ska_array, &input_names, skf_file);
+                delete(&mut ska_array, &input_names, &output_file);
             } else if let Ok(mut ska_array) = MergeSkaArray::<u128>::load(skf_file) {
-                delete(&mut ska_array, &input_names, skf_file);
+                delete(&mut ska_array, &input_names, &output_file);
             } else {
                 panic!("Could not read input file: {skf_file}");
             }


Test patch:
diff --git a/tests/skf_ops.rs b/tests/skf_ops.rs
index 674d2e1..65b1822 100644
--- a/tests/skf_ops.rs
+++ b/tests/skf_ops.rs
@@ -53,8 +53,8 @@ fn merge_delete() {
     Command::new(cargo_bin("ska"))
         .current_dir(sandbox.get_wd())
         .arg("delete")
+        .args(&["-s", "merge.skf"])
         .arg("test_3")
-        .arg("merge.skf")
         .assert()
         .failure();
 
@@ -69,7 +69,7 @@ fn merge_delete() {
     Command::new(cargo_bin("ska"))
         .current_dir(sandbox.get_wd())
         .arg("delete")
-        .arg("merge.skf")
+        .args(&["-s", "merge.skf"])
         .arg("test_2")
         .assert()
         .success();
@@ -125,8 +125,8 @@ fn merge_delete_u128() {
     Command::new(cargo_bin("ska"))
         .current_dir(sandbox.get_wd())
         .arg("delete")
-        .arg("test_3")
-        .arg("merge.skf")
+        .args(&["-s", "merge.skf"])
+        .args(&["-f", &sandbox.file_string("missing_delete.txt", TestDir::Input)])
         .arg("-v")
         .assert()
         .failure();
@@ -142,7 +142,8 @@ fn merge_delete_u128() {
     Command::new(cargo_bin("ska"))
         .current_dir(sandbox.get_wd())
         .arg("delete")
-        .arg("merge.skf")
+        .args(&["-s", "merge.skf"])
+        .args(&["-o", "merge_delete"])
         .arg("test_2")
         .arg("-v")
         .assert()
@@ -151,7 +152,7 @@ fn merge_delete_u128() {
     Command::new(cargo_bin("ska"))
         .current_dir(sandbox.get_wd())
         .arg("nk")
-        .arg("merge.skf")
+        .arg("merge_delete.skf")
         .assert()
         .stdout_eq(test1_nk.stdout);
 }
diff --git a/tests/test_files_in/missing_delete.txt b/tests/test_files_in/missing_delete.txt
new file mode 100644
index 0000000..d279263
--- /dev/null
+++ b/tests/test_files_in/missing_delete.txt
@@ -0,0 +1,1 @@
+test_3