require moltype in param str

sourmash-bio · Nov 16, 2024 · 36649af · 36649af
1 parent 3b8e247
commit 36649af
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 29 deletions.
diff --git a/src/python/tests/test_sketch.py b/src/python/tests/test_sketch.py
@@ -434,29 +434,29 @@ def test_manysketch_bad_fa_csv_4(runtmp, capfd):
     assert "Could not load fromfile csv" in captured.err
 
 
-# def test_manysketch_bad_param_str_moltype(runtmp, capfd):
-#     # no moltype provided in param str
-#     fa_csv = runtmp.output("db-fa.txt")
+def test_manysketch_bad_param_str_moltype(runtmp, capfd):
+    # no moltype provided in param str
+    fa_csv = runtmp.output("db-fa.txt")
 
-#     fa1 = get_test_data("short.fa")
-#     fa2 = get_test_data("short2.fa")
-#     fa3 = get_test_data("short3.fa")
+    fa1 = get_test_data("short.fa")
+    fa2 = get_test_data("short2.fa")
+    fa3 = get_test_data("short3.fa")
 
-#     make_assembly_csv(fa_csv, [fa1, fa2, fa3])
-#     output = runtmp.output("out.zip")
+    make_assembly_csv(fa_csv, [fa1, fa2, fa3])
+    output = runtmp.output("out.zip")
 
-#     with pytest.raises(utils.SourmashCommandFailed):
-#         runtmp.sourmash(
-#             "scripts", "manysketch", fa_csv, "-o", output, "-p", "k=31,scaled=100"
-#         )
+    with pytest.raises(utils.SourmashCommandFailed):
+        runtmp.sourmash(
+            "scripts", "manysketch", fa_csv, "-o", output, "-p", "k=31,scaled=100"
+        )
 
-#     captured = capfd.readouterr()
-#     print(captured.err)
-#     assert (
-#         "Error parsing params string: No moltype provided in params string k=31,scaled=100"
-#         in captured.err
-#     )
-#     assert "Failed to parse params string" in captured.err
+    captured = capfd.readouterr()
+    print(captured.err)
+    assert (
+        "Error parsing params string 'k=31,scaled=100': No moltype provided"
+        in captured.err
+    )
+    assert "Failed to parse params string" in captured.err
 
 
 # def test_manysketch_bad_param_str_ksize(runtmp, capfd):

diff --git a/src/utils/buildutils.rs b/src/utils/buildutils.rs
@@ -552,18 +552,35 @@ impl BuildCollection {
                 _ if item.starts_with("seed=") => {
                     Self::parse_int_once(&item[5..], "seed", &mut seed)?;
                 }
-                _ => return Err(format!("unknown component '{}' in params string", item)),
+                _ => {
+                    return Err(format!(
+                        "Error parsing params string '{}': Unknown component '{}'",
+                        p_str, item
+                    ));
+                }
             }
         }
 
-        // Create a moltype-specific default BuildRecord.
-        let mut base_record = match moltype.as_deref() {
-            Some("dna") => BuildRecord::default_dna(),
-            Some("DNA") => BuildRecord::default_dna(),
-            Some("protein") => BuildRecord::default_protein(),
-            Some("dayhoff") => BuildRecord::default_dayhoff(),
-            Some("hp") => BuildRecord::default_hp(),
-            _ => BuildRecord::default_dna(), // no moltype --> assume DNA
+        // Ensure that moltype was set
+        let moltype = moltype.ok_or_else(|| {
+            format!(
+                "Error parsing params string '{}': No moltype provided",
+                p_str
+            )
+        })?;
+
+        // Create a moltype-specific default BuildRecord or return an error if unsupported.
+        let mut base_record = match moltype.as_str() {
+            "dna" | "DNA" => BuildRecord::default_dna(),
+            "protein" => BuildRecord::default_protein(),
+            "dayhoff" => BuildRecord::default_dayhoff(),
+            "hp" => BuildRecord::default_hp(),
+            _ => {
+                return Err(format!(
+                    "Error parsing params string '{}': Unsupported moltype '{}'",
+                    p_str, moltype
+                ));
+            }
         };
 
         // Apply parsed values
@@ -588,7 +605,10 @@ impl BuildCollection {
         // Ensure that num and scaled are mutually exclusive unless num is 0.
         if let (Some(n), Some(_)) = (num, scaled) {
             if n != 0 {
-                return Err("Cannot specify both 'num' (non-zero) and 'scaled' in the same parameter string".to_string());
+                return Err(format!(
+                    "Error parsing params string '{}': Cannot specify both 'num' (non-zero) and 'scaled' in the same parameter string",
+                    p_str
+                ));
             }
         }