require local urls in seqspec checks

Author: sbooeshaghi

seqspec/seqspec_check.py

@@ -19,6 +19,7 @@
     file_exists,
     load_spec,
     local_onlist_locator,
+    local_resource_url,
     map_read_id_to_regions,
 )
 
@@ -269,7 +270,17 @@ def check_onlist_files_exist(spec, errors, idx):
 
         for ol in olrgns:
             if ol.urltype == "local":
-                locator = local_onlist_locator(ol)
+                try:
+                    locator = local_onlist_locator(ol)
+                except ValueError as err:
+                    errobj = {
+                        "error_type": "check_onlist_files_exist",
+                        "error_message": str(err),
+                        "error_object": "onlist",
+                    }
+                    errors.append(errobj)
+                    idx += 1
+                    continue
                 if locator.endswith(".gz"):
                     check = locator
                     if spec_base and not Path(check).is_absolute():
@@ -350,7 +361,17 @@ def check_read_files_exist(spec, errors, idx):
         for read in spec.sequence_spec:
             for f in read.files:
                 if f.urltype == "local":
-                    check = f.url
+                    try:
+                        check = local_resource_url(f.url, f.filename, "file")
+                    except ValueError as err:
+                        errobj = {
+                            "error_type": "check_read_files_exist",
+                            "error_message": str(err),
+                            "error_object": "file",
+                        }
+                        errors.append(errobj)
+                        idx += 1
+                        continue
                     if spec_base and not Path(check).is_absolute():
                         check = str((spec_base / check).resolve())
                     if not path.exists(check):

seqspec/utils.py

@@ -503,8 +503,14 @@ def yield_onlist_contents(stream):
         yield line.strip().split()[0]
 
 
+def local_resource_url(url: str, filename: str, resource: str) -> str:
+    if not url:
+        raise ValueError(f"local {resource} '{filename}' has empty url")
+    return str(url)
+
+
 def local_onlist_locator(onlist: Onlist) -> str:
-    return str(onlist.url or onlist.filename)
+    return local_resource_url(onlist.url, onlist.filename, "onlist")
 
 
 def read_local_list(onlist: Onlist, base_path: str = "") -> List[str]:

src/seqspec_check.rs

@@ -359,7 +359,19 @@ fn check_onlist_files_exist(
         match ol.urltype.as_str() {
             "local" => {
                 let mut candidates: Vec<PathBuf> = Vec::new();
-                let locator = utils::local_onlist_locator(&ol);
+                let locator = match utils::local_onlist_locator(&ol) {
+                    Ok(locator) => locator,
+                    Err(err) => {
+                        push_error(
+                            &mut errors,
+                            &mut idx,
+                            "check_onlist_files_exist",
+                            err,
+                            "onlist",
+                        );
+                        continue;
+                    }
+                };
                 let p = PathBuf::from(locator);
                 candidates.push(if let Some(base) = spec_base {
                     if p.is_absolute() {
@@ -439,7 +451,20 @@ fn check_read_files_exist(
         for f in &read.files {
             match f.urltype.as_str() {
                 "local" => {
-                    let p = PathBuf::from(&f.url);
+                    let locator = match utils::local_resource_url(&f.url, &f.filename, "file") {
+                        Ok(locator) => locator,
+                        Err(err) => {
+                            push_error(
+                                &mut errors,
+                                &mut idx,
+                                "check_read_files_exist",
+                                err,
+                                "file",
+                            );
+                            continue;
+                        }
+                    };
+                    let p = PathBuf::from(locator);
                     let full = if let Some(base) = spec_base {
                         if p.is_absolute() {
                             p.clone()
@@ -993,6 +1018,40 @@ mod tests {
         );
     }
 
+    #[test]
+    fn test_check_onlist_files_exist_errors_when_local_url_is_empty() {
+        let spec_path = PathBuf::from("tests/fixtures/onlist_read_clip/spec.yaml");
+        let mut spec = load_spec(&spec_path);
+        let barcode_region = spec
+            .library_spec
+            .get_mut(0)
+            .unwrap()
+            .regions
+            .iter_mut()
+            .find(|region| region.region_id == "barcode_a")
+            .unwrap();
+        barcode_region.onlist.as_mut().unwrap().url.clear();
+
+        let diagnostics = seqspec_check(&spec, None, &spec_path);
+        assert!(diagnostics.iter().any(|diagnostic| {
+            diagnostic.error_type == "check_onlist_files_exist"
+                && diagnostic.error_message == "local onlist 'barcode_a.txt' has empty url"
+        }));
+    }
+
+    #[test]
+    fn test_check_read_files_exist_errors_when_local_url_is_empty() {
+        let spec_path = PathBuf::from("tests/fixtures/onlist_read_clip/spec.yaml");
+        let mut spec = load_spec(&spec_path);
+        spec.sequence_spec[0].files[0].url.clear();
+
+        let diagnostics = seqspec_check(&spec, None, &spec_path);
+        assert!(diagnostics.iter().any(|diagnostic| {
+            diagnostic.error_type == "check_read_files_exist"
+                && diagnostic.error_message == "local file 'rna_read.fastq.gz' has empty url"
+        }));
+    }
+
     #[test]
     fn test_error_obj_structure() {
         let e = ErrorObj {

src/seqspec_onlist.rs

@@ -181,7 +181,10 @@ fn get_onlist_urls(onlists: &Vec<Onlist>, base_path: &Path) -> Vec<UrlInfo> {
     for ol in onlists {
         let url = if ol.urltype == "local" {
             base_path
-                .join(utils::local_onlist_locator(ol))
+                .join(utils::local_onlist_locator(ol).unwrap_or_else(|err| {
+                    eprintln!("{}", err);
+                    std::process::exit(1);
+                }))
                 .to_string_lossy()
                 .to_string()
         } else {
@@ -205,7 +208,10 @@ fn download_onlists_to_path(
     let mut out = Vec::new();
     for ol in onlists {
         if ol.urltype == "local" {
-            let local = base_path.join(utils::local_onlist_locator(ol));
+            let local = base_path.join(utils::local_onlist_locator(ol).unwrap_or_else(|err| {
+                eprintln!("{}", err);
+                std::process::exit(1);
+            }));
             out.push(PathInfo {
                 url: local.to_string_lossy().to_string(),
             });
@@ -242,8 +248,11 @@ fn join_onlists_and_save(
     let mut contents: Vec<Vec<String>> = Vec::new();
     for ol in onlists {
         let content = if ol.urltype == "local" {
-            utils::read_local_list(&base_path.join(utils::local_onlist_locator(ol)))
-                .unwrap_or_default()
+            let locator = utils::local_onlist_locator(ol).unwrap_or_else(|err| {
+                eprintln!("{}", err);
+                std::process::exit(1);
+            });
+            utils::read_local_list(&base_path.join(locator)).unwrap_or_default()
         } else {
             utils::read_remote_list(&ol.url, remote_access).unwrap_or_default()
         };

src/utils.rs

@@ -61,14 +61,22 @@ pub fn load_spec(spec: &std::path::PathBuf) -> Assay {
     spec.into_assay()
 }
 
-pub fn local_onlist_locator(onlist: &Onlist) -> &str {
-    if onlist.url.is_empty() {
-        &onlist.filename
+pub fn local_resource_url<'a>(
+    url: &'a str,
+    filename: &str,
+    resource: &str,
+) -> Result<&'a str, String> {
+    if url.is_empty() {
+        Err(format!("local {} '{}' has empty url", resource, filename))
     } else {
-        &onlist.url
+        Ok(url)
     }
 }
 
+pub fn local_onlist_locator(onlist: &Onlist) -> Result<&str, String> {
+    local_resource_url(&onlist.url, &onlist.filename, "onlist")
+}
+
 /// Read a local text file into Vec<String>, handling optional .gz
 pub fn read_local_list(path: &std::path::Path) -> Result<Vec<String>, String> {
     let p = if path.exists() {
@@ -483,11 +491,14 @@ mod tests {
             String::new(),
         );
 
-        assert_eq!(local_onlist_locator(&onlist), "nested/whitelist.txt");
+        assert_eq!(
+            local_onlist_locator(&onlist).unwrap(),
+            "nested/whitelist.txt"
+        );
     }
 
     #[test]
-    fn test_local_onlist_locator_falls_back_to_filename() {
+    fn test_local_onlist_locator_errors_when_url_is_empty() {
         let onlist = Onlist::new(
             "ol1".into(),
             "display.txt".into(),
@@ -498,6 +509,17 @@ mod tests {
             String::new(),
         );
 
-        assert_eq!(local_onlist_locator(&onlist), "display.txt");
+        assert_eq!(
+            local_onlist_locator(&onlist).unwrap_err(),
+            "local onlist 'display.txt' has empty url"
+        );
+    }
+
+    #[test]
+    fn test_local_resource_url_errors_when_url_is_empty() {
+        assert_eq!(
+            local_resource_url("", "display.fastq.gz", "file").unwrap_err(),
+            "local file 'display.fastq.gz' has empty url"
+        );
     }
 }

tests/test_seqspec_check.py

@@ -91,3 +91,34 @@ def test_seqspec_check_prefers_local_onlist_url():
         diagnostic["error_type"] == "check_onlist_files_exist"
         for diagnostic in diagnostics
     )
+
+
+def test_seqspec_check_errors_when_local_onlist_url_is_empty():
+    spec_path = Path("tests/fixtures/onlist_read_clip/spec.yaml")
+    spec = load_spec(spec_path)
+
+    barcode_region = spec.get_libspec("rna").get_region_by_id("barcode_a")[0]
+    barcode_region.onlist.url = ""
+
+    diagnostics = seqspec_check(spec=spec)
+
+    assert any(
+        diagnostic["error_type"] == "check_onlist_files_exist"
+        and diagnostic["error_message"] == "local onlist 'barcode_a.txt' has empty url"
+        for diagnostic in diagnostics
+    )
+
+
+def test_seqspec_check_errors_when_local_file_url_is_empty():
+    spec_path = Path("tests/fixtures/onlist_read_clip/spec.yaml")
+    spec = load_spec(spec_path)
+
+    spec.sequence_spec[0].files[0].url = ""
+
+    diagnostics = seqspec_check(spec=spec)
+
+    assert any(
+        diagnostic["error_type"] == "check_read_files_exist"
+        and diagnostic["error_message"] == "local file 'rna_read.fastq.gz' has empty url"
+        for diagnostic in diagnostics
+    )

tests/test_utils.py

@@ -9,6 +9,7 @@
 
 from seqspec.utils import (
     load_spec_stream,
+    local_resource_url,
     read_local_list,
     read_remote_list,
     get_remote_auth_token,
@@ -286,6 +287,21 @@ def test_local_onlist_locator_prefers_url_when_present():
     assert local_onlist_locator(onlist) == "nested/whitelist.txt"
 
 
+def test_local_onlist_locator_errors_when_url_is_empty():
+    onlist = Onlist(
+        file_id="ol1",
+        filename="display.txt",
+        filetype="txt",
+        filesize=0,
+        url="",
+        urltype="local",
+        md5="",
+    )
+
+    with pytest.raises(ValueError, match="local onlist 'display.txt' has empty url"):
+        local_onlist_locator(onlist)
+
+
 def test_read_local_list_prefers_url_when_present(tmp_path):
     nested = tmp_path / "nested"
     nested.mkdir()
@@ -302,3 +318,8 @@ def test_read_local_list_prefers_url_when_present(tmp_path):
     )
 
     assert read_local_list(onlist, str(tmp_path)) == ["AAAA", "CCCC"]
+
+
+def test_local_resource_url_errors_when_url_is_empty():
+    with pytest.raises(ValueError, match="local file 'display.fastq.gz' has empty url"):
+        local_resource_url("", "display.fastq.gz", "file")