From 5155400a7b5a8f963094adab6969b19446c8a1e7 Mon Sep 17 00:00:00 2001 From: Chris Saunders Date: Fri, 18 Oct 2024 21:39:06 -0700 Subject: [PATCH 1/3] Add bcf record to_vcf_string method and test --- src/bcf/record.rs | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/bcf/record.rs b/src/bcf/record.rs index c1066cd30..c1f703fd1 100644 --- a/src/bcf/record.rs +++ b/src/bcf/record.rs @@ -1088,6 +1088,27 @@ impl Record { } "".to_owned() } + + /// Format the record as a VCF string + /// + /// This method is intended for debug and error reporting purposes. + /// + pub fn to_vcf_string(&self) -> String { + // int vcf_format(const bcf_hdr_t *h, const bcf1_t *v, kstring_t *s) + let mut buf = htslib::kstring_t { + l: 0, + m: 0, + s: ptr::null_mut(), + }; + unsafe { + htslib::vcf_format(self.header().inner, self.inner, &mut buf); + let vcf_str = String::from(ffi::CStr::from_ptr(buf.s).to_str().unwrap()); + if !buf.s.is_null() { + libc::free(buf.s as *mut libc::c_void); + } + vcf_str + } + } } impl Clone for Record { @@ -1714,4 +1735,19 @@ mod tests { assert!(!record.has_filter(&bar)); assert!(record.has_filter("PASS".as_bytes())); } + + #[test] + fn test_record_to_vcf_string() { + let tmp = NamedTempFile::new().unwrap(); + let path = tmp.path(); + let mut header = Header::new(); + // Add contig records + let header_contig_line = b"##contig="; + header.push_record(header_contig_line); + header.push_record(br#"##FILTER="#); + let vcf = Writer::from_path(path, &header, true, Format::Vcf).unwrap(); + let mut record = vcf.empty_record(); + record.push_filter("foo".as_bytes()).unwrap(); + assert_eq!(record.to_vcf_string(), "chr1\t1\t.\t.\t.\t0\tfoo\t.\n"); + } } From 7ee96e260deae4429ad2a056a434ff338ca1a42b Mon Sep 17 00:00:00 2001 From: Chris Saunders Date: Sat, 19 Oct 2024 16:11:20 -0700 Subject: [PATCH 2/3] Reformat as Display trait --- src/bcf/record.rs | 49 ++++++++++++++++++++++------------------------- 1 file changed, 23 insertions(+), 26 deletions(-) diff --git a/src/bcf/record.rs b/src/bcf/record.rs index c1f703fd1..c0ddbf0ad 100644 --- a/src/bcf/record.rs +++ b/src/bcf/record.rs @@ -1088,27 +1088,6 @@ impl Record { } "".to_owned() } - - /// Format the record as a VCF string - /// - /// This method is intended for debug and error reporting purposes. - /// - pub fn to_vcf_string(&self) -> String { - // int vcf_format(const bcf_hdr_t *h, const bcf1_t *v, kstring_t *s) - let mut buf = htslib::kstring_t { - l: 0, - m: 0, - s: ptr::null_mut(), - }; - unsafe { - htslib::vcf_format(self.header().inner, self.inner, &mut buf); - let vcf_str = String::from(ffi::CStr::from_ptr(buf.s).to_str().unwrap()); - if !buf.s.is_null() { - libc::free(buf.s as *mut libc::c_void); - } - vcf_str - } - } } impl Clone for Record { @@ -1258,6 +1237,25 @@ unsafe impl Send for Record {} unsafe impl Sync for Record {} +impl fmt::Display for Record { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let mut buf = htslib::kstring_t { + l: 0, + m: 0, + s: ptr::null_mut(), + }; + let vcf_str = unsafe { + htslib::vcf_format(self.header().inner, self.inner, &mut buf); + let vcf_str = String::from(ffi::CStr::from_ptr(buf.s).to_str().unwrap()); + if !buf.s.is_null() { + libc::free(buf.s as *mut libc::c_void); + } + vcf_str + }; + write!(f, "{vcf_str}") + } +} + /// Info tag representation. #[derive(Debug)] pub struct Info<'a, B: BorrowMut + Borrow> { @@ -1737,17 +1735,16 @@ mod tests { } #[test] - fn test_record_to_vcf_string() { + fn test_record_display_trait() { let tmp = NamedTempFile::new().unwrap(); let path = tmp.path(); let mut header = Header::new(); - // Add contig records - let header_contig_line = b"##contig="; - header.push_record(header_contig_line); + header.push_record(b"##contig="); header.push_record(br#"##FILTER="#); let vcf = Writer::from_path(path, &header, true, Format::Vcf).unwrap(); let mut record = vcf.empty_record(); record.push_filter("foo".as_bytes()).unwrap(); - assert_eq!(record.to_vcf_string(), "chr1\t1\t.\t.\t.\t0\tfoo\t.\n"); + let s = format!("{record}"); + assert_eq!(s, "chr1\t1\t.\t.\t.\t0\tfoo\t.\n"); } } From bdea7de6f29d027bae305ab60d1c294d7d9cf30f Mon Sep 17 00:00:00 2001 From: Chris Saunders Date: Mon, 21 Oct 2024 11:09:59 -0700 Subject: [PATCH 3/3] Revert to Record::to_vcf_string method Added return value checking on vcf_format, which also suggests that the methods should return a Result and shound't be implemented directly as a Display trait. --- src/bcf/record.rs | 69 ++++++++++++++++++++++++++++++++--------------- src/errors.rs | 2 ++ 2 files changed, 49 insertions(+), 22 deletions(-) diff --git a/src/bcf/record.rs b/src/bcf/record.rs index c0ddbf0ad..e84a6273f 100644 --- a/src/bcf/record.rs +++ b/src/bcf/record.rs @@ -1088,6 +1088,38 @@ impl Record { } "".to_owned() } + + /// Convert to VCF String + /// + /// Intended for debug only. Use Writer for efficient VCF output. + /// + pub fn to_vcf_string(&self) -> Result { + let mut buf = htslib::kstring_t { + l: 0, + m: 0, + s: ptr::null_mut(), + }; + let ret = unsafe { htslib::vcf_format(self.header().inner, self.inner, &mut buf) }; + + if ret < 0 { + if !buf.s.is_null() { + unsafe { + libc::free(buf.s as *mut libc::c_void); + } + } + return Err(Error::BcfToString); + } + + let vcf_str = unsafe { + let vcf_str = String::from(ffi::CStr::from_ptr(buf.s).to_str().unwrap()); + if !buf.s.is_null() { + libc::free(buf.s as *mut libc::c_void); + } + vcf_str + }; + + Ok(vcf_str) + } } impl Clone for Record { @@ -1237,25 +1269,6 @@ unsafe impl Send for Record {} unsafe impl Sync for Record {} -impl fmt::Display for Record { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let mut buf = htslib::kstring_t { - l: 0, - m: 0, - s: ptr::null_mut(), - }; - let vcf_str = unsafe { - htslib::vcf_format(self.header().inner, self.inner, &mut buf); - let vcf_str = String::from(ffi::CStr::from_ptr(buf.s).to_str().unwrap()); - if !buf.s.is_null() { - libc::free(buf.s as *mut libc::c_void); - } - vcf_str - }; - write!(f, "{vcf_str}") - } -} - /// Info tag representation. #[derive(Debug)] pub struct Info<'a, B: BorrowMut + Borrow> { @@ -1735,7 +1748,17 @@ mod tests { } #[test] - fn test_record_display_trait() { + fn test_record_to_vcf_string_err() { + let tmp = NamedTempFile::new().unwrap(); + let path = tmp.path(); + let header = Header::new(); + let vcf = Writer::from_path(path, &header, true, Format::Vcf).unwrap(); + let record = vcf.empty_record(); + assert!(record.to_vcf_string().is_err()); + } + + #[test] + fn test_record_to_vcf_string() { let tmp = NamedTempFile::new().unwrap(); let path = tmp.path(); let mut header = Header::new(); @@ -1744,7 +1767,9 @@ mod tests { let vcf = Writer::from_path(path, &header, true, Format::Vcf).unwrap(); let mut record = vcf.empty_record(); record.push_filter("foo".as_bytes()).unwrap(); - let s = format!("{record}"); - assert_eq!(s, "chr1\t1\t.\t.\t.\t0\tfoo\t.\n"); + assert_eq!( + record.to_vcf_string().unwrap(), + "chr1\t1\t.\t.\t.\t0\tfoo\t.\n" + ); } } diff --git a/src/errors.rs b/src/errors.rs index 2e89854f0..82693a381 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -129,6 +129,8 @@ pub enum Error { BcfSetValues, #[error("failed to remove alleles in BCF/VCF record")] BcfRemoveAlleles, + #[error("failed to render BCF record as string")] + BcfToString, #[error("invalid compression level {level}")] BgzfInvalidCompressionLevel { level: i8 },