Merge pull request #58 from anergictcell/refactor/error-handling

Refactor/error handling

Cargo.toml

@@ -36,5 +36,9 @@ harness = false
 name = "ancestors"
 harness = false
 
+[[bench]]
+name = "parser"
+harness = false
+
 [profile.release]
 lto = true

examples/compare_ontologies.rs

@@ -16,7 +16,7 @@ fn ontology(path_arg: &str) -> Ontology {
     }
 }
 
-fn ontology2(path_arg: &str) -> Ontology {
+fn ontology_transitive(path_arg: &str) -> Ontology {
     let path = Path::new(path_arg);
 
     match path.is_file() {
@@ -213,17 +213,21 @@ fn print_replacement_diff(replacements: Option<(Option<HpoTermId>, Option<HpoTer
 fn main() {
     let mut args = std::env::args();
 
-    if args.len() != 3 {
+    if args.len() < 2 {
         println!("Compare two Ontologies to each other and print the differences\n\n");
-        println!("Usage:\ncompare_ontologies </PATH/TO/ONTOLOGY> </PATH/TO/OTHER-ONTOLOGY>");
+        println!("Usage:\ncompare_ontologies </PATH/TO/ONTOLOGY> [</PATH/TO/OTHER-ONTOLOGY>]");
         println!("e.g.:\ncompare_ontologies tests/ontology.hpo tests/ontology_v2.hpo:\n");
+        println!("Alternatively compare transitive vs non-transitive:\ncompare_ontologies tests/ontology.hpo\n");
         process::exit(1)
     }
     let arg_old = args.nth(1).unwrap();
-    let arg_new = args.next().unwrap();
-
     let lhs = ontology(&arg_old);
-    let rhs = ontology2(&arg_new);
+
+    let rhs = if let Some(arg_new) = args.next() {
+        ontology(&arg_new)
+    } else {
+        ontology_transitive(&arg_old)
+    };
 
     let diffs = lhs.compare(&rhs);
 

src/lib.rs

@@ -58,6 +58,9 @@ pub enum HpoError {
     /// Failed to convert an integer to a float
     #[error("cannot convert int to float")]
     TryFromIntError(#[from] std::num::TryFromIntError),
+    /// Failed to parse a line of input data from the JAX obo
+    #[error("invalid input data: {0}")]
+    InvalidInput(String),
 }
 
 impl From<ParseIntError> for HpoError {

src/ontology.rs

@@ -1,5 +1,5 @@
 use core::fmt::Debug;
-use std::collections::hash_map::Values;
+use std::collections::hash_map::{Entry, Values};
 use std::collections::{HashMap, HashSet};
 use std::fs::File;
 use std::io::Read;
@@ -366,7 +366,7 @@ impl Ontology {
     ///
     /// - Actual OBO data: [`hp.obo`](https://hpo.jax.org/app/data/ontology)
     /// - Links between HPO and OMIM diseases: [`phenotype.hpoa`](https://hpo.jax.org/app/data/annotations)
-    /// - Links between HPO and Genes: [`phenotype_to_genes.txt`](http://purl.obolibrary.org/obo/hp/hpoa/phenotype_to_genes.txt)
+    /// - Links between HPO and Genes: [`genes_to_phenotype.txt`](http://purl.obolibrary.org/obo/hp/hpoa/genes_to_phenotype.txt)
     ///
     /// and then specify the folder where the data is stored.
     ///
@@ -424,7 +424,7 @@ impl Ontology {
     ///
     /// - Actual OBO data: [`hp.obo`](https://hpo.jax.org/app/data/ontology)
     /// - Links between HPO and OMIM diseases: [`phenotype.hpoa`](https://hpo.jax.org/app/data/annotations)
-    /// - Links between HPO and Genes: [`genes_to_phenotypes.txt`](http://purl.obolibrary.org/obo/hp/hpoa/genes_to_phenotype.txt)
+    /// - Links between HPO and Genes: [`phenotype_to_genes.txt`](http://purl.obolibrary.org/obo/hp/hpoa/phenotype_to_genes.txt)
     ///
     /// and then specify the folder where the data is stored.
     ///
@@ -898,11 +898,12 @@ impl Ontology {
     ///
     /// ```
     /// use hpo::Ontology;
+    /// use hpo::annotations::GeneId;
     ///
     /// let ontology_1 = Ontology::from_binary("tests/example.hpo").unwrap();
     /// let mut ontology_2 = Ontology::default();
     ///
-    /// ontology_2.add_gene("FOOBAR", "666666").unwrap();
+    /// ontology_2.add_gene("FOOBAR", GeneId::from(666666));
     ///
     /// let compare = ontology_1.compare(&ontology_2);
     /// assert_eq!(compare.added_hpo_terms().len(), 0);
@@ -974,13 +975,13 @@ impl Ontology {
             if matched_terms.is_empty() {
                 continue;
             }
-            let gene_id = ont.add_gene(
+            ont.add_gene(
                 self.gene(gene.id()).ok_or(HpoError::DoesNotExist)?.name(),
-                &gene.id().as_u32().to_string(),
-            )?;
+                *gene.id(),
+            );
             for term in &matched_terms {
-                ont.link_gene_term(term, gene_id)?;
-                ont.gene_mut(&gene_id)
+                ont.link_gene_term(term, *gene.id())?;
+                ont.gene_mut(gene.id())
                     .ok_or(HpoError::DoesNotExist)?
                     .add_term(term);
             }
@@ -1270,7 +1271,7 @@ impl Ontology {
         }
     }
 
-    /// Add a gene to the Ontology. and return the [`GeneId`]
+    /// Add a gene to the Ontology
     ///
     /// If the gene does not yet exist, a new [`Gene`] entity is created
     /// and stored in the Ontology.
@@ -1281,19 +1282,19 @@ impl Ontology {
     /// Adding a gene does not connect it to any HPO terms.
     /// Use [`Ontology::link_gene_term`] for creating connections.
     ///
-    /// # Errors
-    ///
-    /// If the `gene_id` is invalid, an [`HpoError::ParseIntError`] is returned
+    /// This method was changed to receive the `gene_id` as [`GeneId`]
+    /// instead of `str` in `0.10` and does not return a `Result` anymore.
     ///
     /// # Examples
     ///
     /// ```
     /// use hpo::Ontology;
+    /// use hpo::annotations::GeneId;
     ///
     /// let mut ontology = Ontology::default();
     /// assert!(ontology.gene(&1u32.into()).is_none());
     ///
-    /// ontology.add_gene("Foo", "1");
+    /// ontology.add_gene("Foo", GeneId::from(1));
     ///
     /// // Genes can be iterated...
     /// let mut gene_iterator = ontology.genes();
@@ -1304,14 +1305,9 @@ impl Ontology {
     /// // .. or accessed directly
     /// assert!(ontology.gene(&1u32.into()).is_some());
     /// ```
-    pub fn add_gene(&mut self, gene_name: &str, gene_id: &str) -> HpoResult<GeneId> {
-        let id = GeneId::try_from(gene_id)?;
-        match self.genes.entry(id) {
-            std::collections::hash_map::Entry::Occupied(_) => Ok(id),
-            std::collections::hash_map::Entry::Vacant(entry) => {
-                entry.insert(Gene::new(id, gene_name));
-                Ok(id)
-            }
+    pub fn add_gene(&mut self, gene_name: &str, gene_id: GeneId) {
+        if let Entry::Vacant(entry) = self.genes.entry(gene_id) {
+            entry.insert(Gene::new(gene_id, gene_name));
         }
     }
 
@@ -1383,7 +1379,7 @@ impl Ontology {
     ///
     /// let mut ontology = Ontology::default();
     /// ontology.insert_term("Term-Foo".into(), 1u32);
-    /// ontology.add_gene("Foo", "5");
+    /// ontology.add_gene("Foo", GeneId::from(5));
     /// ontology.link_gene_term(1u32, GeneId::from(5u32)).unwrap();
     ///
     /// let term = ontology.hpo(1u32).unwrap();