Skip to content

Commit

Permalink
feat: proper error handling with enums and thiserror (#69)
Browse files Browse the repository at this point in the history
  • Loading branch information
@holtgrewe
holtgrewe committed Apr 21, 2023
1 parent d0fca0f commit 6dd7751
Show file tree
Hide file tree
Showing 20 changed files with 505 additions and 479 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ name = "hgvs"
path = "src/lib.rs"

[dependencies]
anyhow = "1.0.69"
base16ct = "0.2.0"
bio = "1.1.0"
chrono = "0.4.23"
Expand All @@ -32,6 +31,7 @@ rustc-hash = "1.1.0"
seqrepo = { version = "0.3.0" }
serde = { version = "1.0.152", features = ["derive"] }
serde_json = "1.0.93"
thiserror = "1.0"

[dev-dependencies]
csv = "1.2.0"
Expand Down
98 changes: 43 additions & 55 deletions src/data/cdot/json.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ pub struct Provider {
}

impl Provider {
pub fn new(config: Config) -> Result<Self, anyhow::Error> {
pub fn new(config: Config) -> Result<Self, Error> {
let seqrepo = PathBuf::from(&config.seqrepo_path);
let path = seqrepo
.parent()
Expand Down Expand Up @@ -83,7 +83,7 @@ impl Provider {
pub fn with_seqrepo(
config: Config,
seqrepo: Rc<dyn SeqRepoInterface>,
) -> Result<Provider, anyhow::Error> {
) -> Result<Provider, Error> {
Ok(Self {
inner: TxProvider::with_config(
config
Expand Down Expand Up @@ -114,11 +114,11 @@ impl ProviderInterface for Provider {
self.inner.get_assembly_map(assembly)
}

fn get_gene_info(&self, hgnc: &str) -> Result<GeneInfoRecord, anyhow::Error> {
fn get_gene_info(&self, hgnc: &str) -> Result<GeneInfoRecord, Error> {
self.inner.get_gene_info(hgnc)
}

fn get_pro_ac_for_tx_ac(&self, tx_ac: &str) -> Result<Option<String>, anyhow::Error> {
fn get_pro_ac_for_tx_ac(&self, tx_ac: &str) -> Result<Option<String>, Error> {
self.inner.get_pro_ac_for_tx_ac(tx_ac)
}

Expand All @@ -127,7 +127,7 @@ impl ProviderInterface for Provider {
ac: &str,
begin: Option<usize>,
end: Option<usize>,
) -> Result<String, anyhow::Error> {
) -> Result<String, Error> {
self.seqrepo.fetch_sequence_part(
&seqrepo::AliasOrSeqId::Alias {
value: ac.to_string(),
Expand All @@ -138,14 +138,11 @@ impl ProviderInterface for Provider {
)
}

fn get_acs_for_protein_seq(&self, seq: &str) -> Result<Vec<String>, anyhow::Error> {
fn get_acs_for_protein_seq(&self, seq: &str) -> Result<Vec<String>, Error> {
self.inner.get_acs_for_protein_seq(seq)
}

fn get_similar_transcripts(
&self,
tx_ac: &str,
) -> Result<Vec<TxSimilarityRecord>, anyhow::Error> {
fn get_similar_transcripts(&self, tx_ac: &str) -> Result<Vec<TxSimilarityRecord>, Error> {
self.inner.get_similar_transcripts(tx_ac)
}

Expand All @@ -154,11 +151,11 @@ impl ProviderInterface for Provider {
tx_ac: &str,
alt_ac: &str,
alt_aln_method: &str,
) -> Result<Vec<TxExonsRecord>, anyhow::Error> {
) -> Result<Vec<TxExonsRecord>, Error> {
self.inner.get_tx_exons(tx_ac, alt_ac, alt_aln_method)
}

fn get_tx_for_gene(&self, gene: &str) -> Result<Vec<TxInfoRecord>, anyhow::Error> {
fn get_tx_for_gene(&self, gene: &str) -> Result<Vec<TxInfoRecord>, Error> {
self.inner.get_tx_for_gene(gene)
}

Expand All @@ -168,12 +165,12 @@ impl ProviderInterface for Provider {
alt_aln_method: &str,
start_i: i32,
end_i: i32,
) -> Result<Vec<TxForRegionRecord>, anyhow::Error> {
) -> Result<Vec<TxForRegionRecord>, Error> {
self.inner
.get_tx_for_region(alt_ac, alt_aln_method, start_i, end_i)
}

fn get_tx_identity_info(&self, tx_ac: &str) -> Result<TxIdentityInfo, anyhow::Error> {
fn get_tx_identity_info(&self, tx_ac: &str) -> Result<TxIdentityInfo, Error> {
self.inner.get_tx_identity_info(tx_ac)
}

Expand All @@ -182,14 +179,11 @@ impl ProviderInterface for Provider {
tx_ac: &str,
alt_ac: &str,
alt_aln_method: &str,
) -> Result<TxInfoRecord, anyhow::Error> {
) -> Result<TxInfoRecord, Error> {
self.inner.get_tx_info(tx_ac, alt_ac, alt_aln_method)
}

fn get_tx_mapping_options(
&self,
tx_ac: &str,
) -> Result<Vec<TxMappingOptionsRecord>, anyhow::Error> {
fn get_tx_mapping_options(&self, tx_ac: &str) -> Result<Vec<TxMappingOptionsRecord>, Error> {
self.inner.get_tx_mapping_options(tx_ac)
}
}
Expand Down Expand Up @@ -546,7 +540,7 @@ struct TxProvider {

/// "Normal" associated functions and methods.
impl TxProvider {
fn with_config(json_paths: &[&str]) -> Result<Self, anyhow::Error> {
fn with_config(json_paths: &[&str]) -> Result<Self, Error> {
let mut genes = HashMap::new();
let mut transcripts = HashMap::new();
let mut transcript_ids_for_gene = HashMap::new();
Expand Down Expand Up @@ -582,7 +576,7 @@ impl TxProvider {
transcript_ids_for_gene: &mut HashMap<String, Vec<String>>,
genes: &mut HashMap<String, models::Gene>,
transcripts: &mut HashMap<String, models::Transcript>,
) -> Result<(), anyhow::Error> {
) -> Result<(), Error> {
log::debug!("Loading cdot transcripts from {:?}", json_path);
let start = Instant::now();
let models::Container {
Expand Down Expand Up @@ -721,7 +715,7 @@ impl TxProvider {
)
}

fn get_gene_info(&self, hgnc: &str) -> Result<GeneInfoRecord, anyhow::Error> {
fn get_gene_info(&self, hgnc: &str) -> Result<GeneInfoRecord, Error> {
let gene = self
.genes
.get(hgnc)
Expand Down Expand Up @@ -749,7 +743,7 @@ impl TxProvider {
})
}

fn get_pro_ac_for_tx_ac(&self, tx_ac: &str) -> Result<Option<String>, anyhow::Error> {
fn get_pro_ac_for_tx_ac(&self, tx_ac: &str) -> Result<Option<String>, Error> {
let transcript = self
.transcripts
.get(tx_ac)
Expand All @@ -762,7 +756,7 @@ impl TxProvider {
/// This is not implemented. The only caller has comment: 'TODO: drop get_acs_for_protein_seq'
/// And is only ever called as a backup when get_pro_ac_for_tx_ac fails
#[allow(dead_code)]
fn get_acs_for_protein_seq(&self, _seq: &str) -> Result<Vec<String>, anyhow::Error> {
fn get_acs_for_protein_seq(&self, _seq: &str) -> Result<Vec<String>, Error> {
log::warn!(
"cdot::data::json::TxProvider::get_acs_for_protein_seq() \
This has not been implemented"
Expand All @@ -775,10 +769,7 @@ impl TxProvider {
/// UTA specific functionality that uses tx_similarity_v table
/// This is not used by the HGVS library
#[allow(dead_code)]
fn get_similar_transcripts(
&self,
_tx_ac: &str,
) -> Result<Vec<TxSimilarityRecord>, anyhow::Error> {
fn get_similar_transcripts(&self, _tx_ac: &str) -> Result<Vec<TxSimilarityRecord>, Error> {
log::warn!(
"cdot::data::json::TxProvider::get_similar_transcripts() \
This has not been implemented"
Expand All @@ -791,7 +782,7 @@ impl TxProvider {
tx_ac: &str,
alt_ac: &str,
alt_aln_method: &str,
) -> Result<Vec<TxExonsRecord>, anyhow::Error> {
) -> Result<Vec<TxExonsRecord>, Error> {
let tx = self.transcripts.get(tx_ac).ok_or(anyhow::anyhow!(
"Could not find transcript for {:?}",
&tx_ac
Expand Down Expand Up @@ -842,7 +833,7 @@ impl TxProvider {
}
}

fn get_tx_for_gene(&self, gene: &str) -> Result<Vec<TxInfoRecord>, anyhow::Error> {
fn get_tx_for_gene(&self, gene: &str) -> Result<Vec<TxInfoRecord>, Error> {
if let Some(tx_acs) = self.transcript_ids_for_gene.get(gene) {
let mut tmp = Vec::new();
for tx_ac in tx_acs {
Expand Down Expand Up @@ -895,7 +886,7 @@ impl TxProvider {
alt_aln_method: &str,
start_i: i32,
end_i: i32,
) -> Result<Vec<TxForRegionRecord>, anyhow::Error> {
) -> Result<Vec<TxForRegionRecord>, Error> {
if alt_aln_method != NCBI_ALN_METHOD {
return Ok(Vec::new());
}
Expand Down Expand Up @@ -950,7 +941,7 @@ impl TxProvider {
}
}

fn get_tx_identity_info(&self, tx_ac: &str) -> Result<TxIdentityInfo, anyhow::Error> {
fn get_tx_identity_info(&self, tx_ac: &str) -> Result<TxIdentityInfo, Error> {
let tx = self.transcripts.get(tx_ac).ok_or(anyhow::anyhow!(
"Could not find transcript for {:?}",
&tx_ac
Expand Down Expand Up @@ -990,7 +981,7 @@ impl TxProvider {
tx_ac: &str,
alt_ac: &str,
alt_aln_method: &str,
) -> Result<TxInfoRecord, anyhow::Error> {
) -> Result<TxInfoRecord, Error> {
let tx = self.transcripts.get(tx_ac).ok_or(anyhow::anyhow!(
"Could not find transcript for {:?}",
&tx_ac
Expand All @@ -1010,10 +1001,7 @@ impl TxProvider {
})
}

fn get_tx_mapping_options(
&self,
tx_ac: &str,
) -> Result<Vec<TxMappingOptionsRecord>, anyhow::Error> {
fn get_tx_mapping_options(&self, tx_ac: &str) -> Result<Vec<TxMappingOptionsRecord>, Error> {
let tx = self.transcripts.get(tx_ac).ok_or(anyhow::anyhow!(
"Could not find transcript for {:?}",
&tx_ac
Expand All @@ -1039,7 +1027,7 @@ pub mod test_helpers {
use super::{Config, Provider};
use seqrepo::{CacheReadingSeqRepo, Interface as SeqRepoInterface};

pub fn build_provider() -> Result<Provider, anyhow::Error> {
pub fn build_provider() -> Result<Provider, Error> {
let sr_cache_mode = std::env::var("TEST_SEQREPO_CACHE_MODE")
.expect("Environment variable TEST_SEQREPO_CACHE_MODE undefined!");
let sr_cache_path = std::env::var("TEST_SEQREPO_CACHE_PATH")
Expand Down Expand Up @@ -1091,7 +1079,7 @@ pub mod tests {
use crate::static_data::Assembly;

#[test]
fn deserialize_brca1() -> Result<(), anyhow::Error> {
fn deserialize_brca1() -> Result<(), Error> {
let json = std::fs::read_to_string(
"tests/data/data/cdot/cdot-0.2.12.refseq.grch37_grch38.brca1.json",
)?;
Expand All @@ -1116,7 +1104,7 @@ pub mod tests {
/// Deserialization of the big cdot files for benchmarking.
#[cfg(deserialization_tests)]
#[test]
fn deserialize_big_files() -> Result<(), anyhow::Error> {
fn deserialize_big_files() -> Result<(), Error> {
let before = std::time::Instant::now();
println!("ensembl...");
let _ensembl: Container =
Expand All @@ -1137,13 +1125,13 @@ pub mod tests {
}

#[test]
fn provider_brca1_smoke() -> Result<(), anyhow::Error> {
fn provider_brca1_smoke() -> Result<(), Error> {
build_provider()?;
Ok(())
}

#[test]
fn provider_versions() -> Result<(), anyhow::Error> {
fn provider_versions() -> Result<(), Error> {
let provider = build_provider()?;

assert_eq!(provider.data_version(), "1.1");
Expand All @@ -1153,7 +1141,7 @@ pub mod tests {
}

#[test]
fn provider_get_assembly_map() -> Result<(), anyhow::Error> {
fn provider_get_assembly_map() -> Result<(), Error> {
let provider = build_provider()?;
assert_eq!(provider.get_assembly_map(Assembly::Grch37p10).len(), 275);
assert_eq!(provider.get_assembly_map(Assembly::Grch38).len(), 455);
Expand All @@ -1162,7 +1150,7 @@ pub mod tests {
}

#[test]
fn provider_get_gene_info() -> Result<(), anyhow::Error> {
fn provider_get_gene_info() -> Result<(), Error> {
let provider = build_provider()?;

assert!(provider.get_gene_info("BRCA2").is_err());
Expand Down Expand Up @@ -1193,7 +1181,7 @@ pub mod tests {
}

#[test]
fn provider_get_pro_ac_for_tx_ac() -> Result<(), anyhow::Error> {
fn provider_get_pro_ac_for_tx_ac() -> Result<(), Error> {
let provider = build_provider()?;

assert!(provider.get_pro_ac_for_tx_ac("NM_007294.0").is_err());
Expand All @@ -1207,7 +1195,7 @@ pub mod tests {
}

#[test]
fn provider_get_acs_for_protein_seq() -> Result<(), anyhow::Error> {
fn provider_get_acs_for_protein_seq() -> Result<(), Error> {
let provider = build_provider()?;

assert_eq!(
Expand All @@ -1219,7 +1207,7 @@ pub mod tests {
}

#[test]
fn provider_get_similar_transcripts() -> Result<(), anyhow::Error> {
fn provider_get_similar_transcripts() -> Result<(), Error> {
let provider = build_provider()?;

assert_eq!(
Expand All @@ -1231,7 +1219,7 @@ pub mod tests {
}

#[test]
fn provider_get_tx_exons() -> Result<(), anyhow::Error> {
fn provider_get_tx_exons() -> Result<(), Error> {
let provider = build_provider()?;

let result = provider.get_tx_exons("NM_007294.3", "NC_000017.10", "splign")?;
Expand Down Expand Up @@ -1705,7 +1693,7 @@ pub mod tests {
}

#[test]
fn provider_get_tx_for_gene() -> Result<(), anyhow::Error> {
fn provider_get_tx_for_gene() -> Result<(), Error> {
let provider = build_provider()?;

let result = provider.get_tx_for_gene("BRCA1")?;
Expand Down Expand Up @@ -1999,7 +1987,7 @@ pub mod tests {
}

#[test]
fn provider_get_tx_for_region_empty() -> Result<(), anyhow::Error> {
fn provider_get_tx_for_region_empty() -> Result<(), Error> {
let provider = build_provider()?;

let result =
Expand All @@ -2013,7 +2001,7 @@ pub mod tests {
}

#[test]
fn provider_get_tx_for_region_brca1() -> Result<(), anyhow::Error> {
fn provider_get_tx_for_region_brca1() -> Result<(), Error> {
let provider = build_provider()?;

let result = provider.get_tx_for_region("NC_000017.10", "splign", 41196311, 41197819)?;
Expand Down Expand Up @@ -2115,7 +2103,7 @@ pub mod tests {
}

#[test]
fn provider_get_tx_info() -> Result<(), anyhow::Error> {
fn provider_get_tx_info() -> Result<(), Error> {
let provider = build_provider()?;

let result = provider.get_tx_info("NM_007294.3", "NC_000017.10", "splign")?;
Expand All @@ -2135,7 +2123,7 @@ pub mod tests {
}

#[test]
fn provider_get_tx_mapping_options() -> Result<(), anyhow::Error> {
fn provider_get_tx_mapping_options() -> Result<(), Error> {
let provider = build_provider()?;

let result = provider.get_tx_mapping_options("NM_007294.3")?;
Expand All @@ -2158,7 +2146,7 @@ pub mod tests {
Ok(())
}

fn build_mapper_37(normalize: bool) -> Result<Mapper, anyhow::Error> {
fn build_mapper_37(normalize: bool) -> Result<Mapper, Error> {
let provider = Rc::new(build_provider()?);
let config = AssemblyMapperConfig {
assembly: Assembly::Grch37,
Expand All @@ -2169,7 +2157,7 @@ pub mod tests {
}

#[test]
fn mapper_brca1_g_c() -> Result<(), anyhow::Error> {
fn mapper_brca1_g_c() -> Result<(), Error> {
let mapper = build_mapper_37(false)?;
let hgvs_g = "NC_000017.10:g.41197701G>C";
let hgvs_n = "NM_007294.4:n.5699C>G";
Expand Down
Loading

0 comments on commit 6dd7751

Please sign in to comment.