further improvements to argument parsing

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "alevin-fry"
-version = "0.6.0"
+version = "0.6.1"
 authors = [
   "Avi Srivastava <avi.srivastava@nyu.edu>",
   "Hirak Sarkar <hirak_sarkar@hms.harvard.edu>",

src/cellfilter.rs

@@ -29,6 +29,7 @@ use std::collections::HashMap;
 use std::fs::File;
 use std::io::{BufRead, BufReader, Read};
 use std::io::{BufWriter, Write};
+use std::path::PathBuf;
 use std::time::Instant;
 
 #[derive(Debug)]
@@ -223,14 +224,14 @@ fn process_unfiltered(
     ft_vals: &rad_types::FileTags,
     filter_meth: &CellFilterMethod,
     expected_ori: Strand,
-    output_dir: &str,
+    output_dir: &PathBuf,
     version: &str,
     max_ambiguity_read: usize,
     velo_mode: bool,
     cmdline: &str,
     log: &slog::Logger,
 ) -> anyhow::Result<u64> {
-    let parent = std::path::Path::new(&output_dir);
+    let parent = std::path::Path::new(output_dir);
     std::fs::create_dir_all(&parent)
         .with_context(|| format!("couldn't create directory path {}", parent.display()))?;
 
@@ -370,7 +371,7 @@ fn process_unfiltered(
         not_found.to_formatted_string(&Locale::en)
     );
 
-    let parent = std::path::Path::new(&output_dir);
+    let parent = std::path::Path::new(output_dir);
     std::fs::create_dir_all(&parent).with_context(|| {
         format!(
             "couldn't create path to output directory {}",
@@ -444,7 +445,7 @@ fn process_filtered(
     ft_vals: &rad_types::FileTags,
     filter_meth: &CellFilterMethod,
     expected_ori: Strand,
-    output_dir: &str,
+    output_dir: &PathBuf,
     version: &str,
     max_ambiguity_read: usize,
     velo_mode: bool,
@@ -519,7 +520,7 @@ fn process_filtered(
         }
     }
 
-    let parent = std::path::Path::new(&output_dir);
+    let parent = std::path::Path::new(output_dir);
     std::fs::create_dir_all(&parent).with_context(|| {
         format!(
             "failed to create path to output location {}",
@@ -594,8 +595,14 @@ pub fn generate_permit_list(gpl_opts: GenPermitListOpts) -> anyhow::Result<u64>
 
     let i_dir = std::path::Path::new(&rad_dir);
 
+    // should we assume this condition was already checked
+    // during parsing?
     if !i_dir.exists() {
-        crit!(log, "the input RAD path {} does not exist", rad_dir);
+        crit!(
+            log,
+            "the input RAD path {} does not exist",
+            rad_dir.display()
+        );
         // std::process::exit(1);
         return Err(anyhow!("execution terminated unexpectedly"));
     }
@@ -715,7 +722,7 @@ pub fn generate_permit_list(gpl_opts: GenPermitListOpts) -> anyhow::Result<u64>
                     &ft_vals,
                     &filter_meth,
                     expected_ori,
-                    &output_dir,
+                    output_dir,
                     version,
                     max_ambiguity_read,
                     velo_mode,
@@ -744,7 +751,7 @@ pub fn generate_permit_list(gpl_opts: GenPermitListOpts) -> anyhow::Result<u64>
                 &ft_vals,
                 &filter_meth,
                 expected_ori,
-                &output_dir,
+                output_dir,
                 version,
                 max_ambiguity_read,
                 velo_mode,

src/collate.rs

@@ -26,15 +26,16 @@ use std::fs::File;
 use std::io::BufReader;
 use std::io::{BufWriter, Cursor, Read, Seek, SeekFrom, Write};
 use std::iter::FromIterator;
+use std::path::PathBuf;
 use std::str::FromStr;
 use std::sync::atomic::{AtomicUsize, Ordering};
 use std::sync::{Arc, Mutex};
 use std::thread;
 
 #[allow(clippy::too_many_arguments)]
 pub fn collate(
-    input_dir: String,
-    rad_dir: String,
+    input_dir: &PathBuf,
+    rad_dir: &PathBuf,
     num_threads: u32,
     max_records: u32,
     compress_out: bool,
@@ -43,7 +44,7 @@ pub fn collate(
     //expected_ori: Strand,
     log: &slog::Logger,
 ) -> anyhow::Result<()> {
-    let parent = std::path::Path::new(&input_dir);
+    let parent = std::path::Path::new(input_dir);
 
     // open the metadata file and read the json
     let gpl_path = parent.join("generate_permit_list.json");
@@ -246,8 +247,8 @@ fn correct_unmapped_counts(
 
 #[allow(clippy::too_many_arguments)]
 pub fn collate_with_temp(
-    input_dir: String,
-    rad_dir: String,
+    input_dir: &PathBuf,
+    rad_dir: &PathBuf,
     num_threads: u32,
     max_records: u32,
     tsv_map: Vec<(u64, u64)>,
@@ -260,7 +261,7 @@ pub fn collate_with_temp(
     // the number of corrected cells we'll write
     let expected_output_chunks = tsv_map.len() as u64;
     // the parent input directory
-    let parent = std::path::Path::new(&input_dir);
+    let parent = std::path::Path::new(input_dir);
 
     let n_workers = if num_threads > 1 {
         (num_threads - 1) as usize
@@ -339,10 +340,14 @@ pub fn collate_with_temp(
         .with_context(|| format!("couldn't create directory {}", cfname))?;
     let owriter = Arc::new(Mutex::new(BufWriter::with_capacity(1048576, ofile)));
 
-    let i_dir = std::path::Path::new(&rad_dir);
+    let i_dir = std::path::Path::new(rad_dir);
 
     if !i_dir.exists() {
-        crit!(log, "the input RAD path {} does not exist", rad_dir);
+        crit!(
+            log,
+            "the input RAD path {} does not exist",
+            rad_dir.display()
+        );
         return Err(anyhow!("invalid input"));
     }
 

src/convert.rs

@@ -23,7 +23,7 @@ use rust_htslib::bam::HeaderView;
 use rust_htslib::{bam, bam::record::Aux, bam::Read};
 use std::collections::HashMap;
 use std::error::Error;
-use std::path::Path;
+use std::path::{Path, PathBuf};
 use std::str;
 
 // pub fn reset_signal_pipe_handler() -> Result<()> {
@@ -92,15 +92,15 @@ pub fn tid_2_contig(h: &HeaderView) -> HashMap<u32, String> {
     dict
 }
 
-pub fn bam2rad(input_file: String, rad_file: String, num_threads: u32, log: &slog::Logger) {
-    let oname = Path::new(&rad_file);
+pub fn bam2rad(input_file: &PathBuf, rad_file: &PathBuf, num_threads: u32, log: &slog::Logger) {
+    let oname = Path::new(rad_file);
     let parent = oname.parent().unwrap();
     std::fs::create_dir_all(&parent).unwrap();
 
     if oname.exists() {
         std::fs::remove_file(oname).expect("could not be deleted");
     }
-    let ofile = File::create(&rad_file).unwrap();
+    let ofile = File::create(rad_file).unwrap();
 
     let mut bam = bam::Reader::from_path(&input_file).unwrap();
     let bam_bytes = fs::metadata(&input_file).unwrap().len();
@@ -487,11 +487,11 @@ pub fn bam2rad(input_file: String, rad_file: String, num_threads: u32, log: &slo
     info!(log, "finished writing to {:?}.", rad_file);
 }
 
-pub fn view(rad_file: String, print_header: bool, out_file: String, log: &slog::Logger) {
+pub fn view(rad_file: &PathBuf, print_header: bool, out_file: String, log: &slog::Logger) {
     let _read_num = view2(rad_file, print_header, out_file, log).unwrap();
 }
 pub fn view2(
-    rad_file: String,
+    rad_file: &PathBuf,
     print_header: bool,
     _out_file: String,
     log: &slog::Logger,

src/main.rs

@@ -17,7 +17,7 @@ use mimalloc::MiMalloc;
 use rand::Rng;
 use slog::{crit, o, warn, Drain};
 use std::borrow::ToOwned;
-use std::path::Path;
+use std::path::{Path, PathBuf};
 
 use alevin_fry::cellfilter::{generate_permit_list, CellFilterMethod};
 use alevin_fry::prog_opts::{GenPermitListOpts, QuantOpts};
@@ -58,6 +58,22 @@ fn file_exists_validator(v: &str) -> Result<String, String> {
     }
 }
 
+/// Checks if the path pointed to by v exists.  It can be
+/// any valid entity (e.g. disk file, FIFO, directory, etc.).
+/// If there is any issue with permissions or failure to properly
+/// resolve symlinks, or if the path is wrong, it returns
+/// an Err(String), else Ok(PathBuf).
+fn pathbuf_file_exists_validator(v: &str) -> Result<PathBuf, String> {
+    // NOTE: we explicitly *do not* check `is_file()` here
+    // since we want to return true even if the path is to
+    // a FIFO/named pipe.
+    if !Path::new(v).exists() {
+        Err(String::from("No valid file was found at this path."))
+    } else {
+        Ok(PathBuf::from(v))
+    }
+}
+
 /// Checks if the path pointed to by v exists and is
 /// a valid directory on disk.  If there is any issue
 /// with permissions or failure to properly
@@ -71,6 +87,19 @@ fn directory_exists_validator(v: &str) -> Result<String, String> {
     }
 }
 
+/// Checks if the path pointed to by v exists and is
+/// a valid directory on disk.  If there is any issue
+/// with permissions or failure to properly
+/// resolve symlinks, or if the path is wrong, it returns
+/// an Err(String), else Ok(PathBuf).
+fn pathbuf_directory_exists_validator(v: &str) -> Result<PathBuf, String> {
+    if !Path::new(v).is_dir() {
+        Err(String::from("No valid directory was found at this path."))
+    } else {
+        Ok(PathBuf::from(v))
+    }
+}
+
 fn main() -> anyhow::Result<()> {
     let num_hardware_threads = num_cpus::get() as u32;
     let max_num_threads: String = (num_cpus::get() as u32).to_string();
@@ -86,19 +115,22 @@ fn main() -> anyhow::Result<()> {
         .about("Convert a BAM file to a RAD file")
         .version(version)
         .author(crate_authors)
-        .arg(arg!(-b --bam <BAMFILE> "input SAM/BAM file").value_parser(file_exists_validator))
+        .arg(
+            arg!(-b --bam <BAMFILE> "input SAM/BAM file")
+                .value_parser(pathbuf_file_exists_validator),
+        )
         .arg(
             arg!(-t --threads <THREADS> "number of threads to use for processing")
                 .value_parser(value_parser!(u32))
                 .default_value(&max_num_threads),
         )
-        .arg(arg!(-o --output <RADFILE> "output RAD file"));
+        .arg(arg!(-o --output <RADFILE> "output RAD file").value_parser(value_parser!(PathBuf)));
 
     let view_app = Command::new("view")
         .about("View a RAD file")
         .version(version)
         .author(crate_authors)
-        .arg(arg!(-r --rad <RADFILE> "input RAD file").value_parser(file_exists_validator))
+        .arg(arg!(-r --rad <RADFILE> "input RAD file").value_parser(pathbuf_file_exists_validator))
         .arg(
             arg!(-H --header "flag for printing header")
                 .takes_value(false)
@@ -111,11 +143,11 @@ fn main() -> anyhow::Result<()> {
         .version(version)
         .author(crate_authors)
         .arg(arg!(-i --input <INPUT>  "input directory containing the map.rad RAD file")
-            .value_parser(directory_exists_validator))
+            .value_parser(pathbuf_directory_exists_validator))
         .arg(arg!(-d --"expected-ori" <EXPECTEDORI> "the expected orientation of alignments")
              .ignore_case(true)
              .value_parser(["fw", "rc", "both", "either"]))
-        .arg(arg!(-o --"output-dir" <OUTPUTDIR>  "output directory"))
+        .arg(arg!(-o --"output-dir" <OUTPUTDIR>  "output directory").value_parser(value_parser!(PathBuf)))
         .arg(arg!(
             -k --"knee-distance"  "attempt to determine the number of barcodes to keep using the knee distance method."
             ).conflicts_with_all(&["force-cell", "valid-bc", "expect-cells", "unfiltered-pl"])
@@ -150,9 +182,9 @@ fn main() -> anyhow::Result<()> {
     .version(version)
     .author(crate_authors)
     .arg(arg!(-i --"input-dir" <INPUTDIR> "input directory made by generate-permit-list")
-        .value_parser(directory_exists_validator))
+        .value_parser(pathbuf_directory_exists_validator))
     .arg(arg!(-r --"rad-dir" <RADFILE> "the directory containing the RAD file to be collated")
-        .value_parser(directory_exists_validator))
+        .value_parser(pathbuf_directory_exists_validator))
     .arg(arg!(-t --threads <THREADS> "number of threads to use for processing").value_parser(value_parser!(u32)).default_value(&max_num_collate_threads))
     .arg(arg!(-c --compress "compress the output collated RAD file").takes_value(false).required(false))
     .arg(arg!(-m --"max-records" <MAXRECORDS> "the maximum number of read records to keep in memory at once")
@@ -260,14 +292,10 @@ fn main() -> anyhow::Result<()> {
     // You can handle information about subcommands by requesting their matches by name
     // (as below), requesting just the name used, or both at the same time
     if let Some(t) = opts.subcommand_matches("generate-permit-list") {
-        let input_dir: String = t
-            .get_one::<String>("input")
-            .expect("no input directory specified")
-            .clone();
-        let output_dir: String = t
-            .get_one::<String>("output-dir")
-            .expect("no input directory specified")
-            .clone();
+        let input_dir: &PathBuf = t.get_one("input").expect("no input directory specified");
+        let output_dir: &PathBuf = t
+            .get_one("output-dir")
+            .expect("no output directory specified");
 
         let valid_ori: bool;
         let expected_ori = match t
@@ -379,15 +407,15 @@ fn main() -> anyhow::Result<()> {
     // convert a BAM file, in *transcriptomic coordinates*, with
     // the appropriate barcode and umi tags, into a RAD file
     if let Some(t) = opts.subcommand_matches("convert") {
-        let input_file: String = t.get_one::<String>("bam").unwrap().clone();
-        let rad_file: String = t.get_one::<String>("output").unwrap().clone();
+        let input_file: &PathBuf = t.get_one("bam").unwrap();
+        let rad_file: &PathBuf = t.get_one("output").unwrap();
         let num_threads: u32 = *t.get_one("threads").unwrap();
         alevin_fry::convert::bam2rad(input_file, rad_file, num_threads, &log)
     }
 
     // convert a rad file to a textual representation and write to stdout
     if let Some(t) = opts.subcommand_matches("view") {
-        let rad_file: String = t.get_one::<String>("rad").unwrap().clone();
+        let rad_file: &PathBuf = t.get_one("rad").unwrap();
         let print_header = t.is_present("header");
         let mut out_file: String = String::from("");
         if t.is_present("output") {
@@ -399,8 +427,8 @@ fn main() -> anyhow::Result<()> {
     // collate a rad file to group together all records corresponding
     // to the same corrected barcode.
     if let Some(t) = opts.subcommand_matches("collate") {
-        let input_dir: String = t.get_one::<String>("input-dir").unwrap().clone();
-        let rad_dir: String = t.get_one::<String>("rad-dir").unwrap().clone();
+        let input_dir: &PathBuf = t.get_one("input-dir").unwrap();
+        let rad_dir: &PathBuf = t.get_one("rad-dir").unwrap();
         let num_threads = *t.get_one("threads").unwrap();
         let compress_out = t.is_present("compress");
         let max_records: u32 = *t.get_one("max-records").unwrap();

src/prog_opts.rs

@@ -6,6 +6,8 @@ use typed_builder::TypedBuilder;
 use crate::cellfilter::CellFilterMethod;
 use crate::quant::{ResolutionStrategy, SplicedAmbiguityModel};
 
+use std::path::PathBuf;
+
 #[derive(TypedBuilder, Debug)]
 //#[builder(name = "QuantOptsBuilder")]
 pub struct QuantOpts<'b, 'c, 'd> {
@@ -30,13 +32,13 @@ pub struct QuantOpts<'b, 'c, 'd> {
 }
 
 #[derive(TypedBuilder, Debug)]
-pub struct GenPermitListOpts<'a, 'b, 'c> {
-    pub input_dir: String,
-    pub output_dir: String,
+pub struct GenPermitListOpts<'a, 'b, 'c, 'd, 'e> {
+    pub input_dir: &'a PathBuf,
+    pub output_dir: &'b PathBuf,
     pub fmeth: CellFilterMethod,
     pub expected_ori: Strand,
     pub velo_mode: bool,
-    pub cmdline: &'a str,
-    pub version: &'b str,
-    pub log: &'c slog::Logger,
+    pub cmdline: &'c str,
+    pub version: &'d str,
+    pub log: &'e slog::Logger,
 }