From 619117240eb861175c1a24d8f159130890114528 Mon Sep 17 00:00:00 2001 From: Petras Date: Fri, 10 May 2024 13:35:34 +0300 Subject: [PATCH 1/2] Bugfix: actual Python exception instead of panic --- src/formats/json.rs | 62 ++++++++++++++++++++++++++++++--------------- test_python.py | 24 ++++++++++++++++++ 2 files changed, 66 insertions(+), 20 deletions(-) diff --git a/src/formats/json.rs b/src/formats/json.rs index be8caa1..ccdccb7 100644 --- a/src/formats/json.rs +++ b/src/formats/json.rs @@ -320,7 +320,13 @@ pub fn save<'t, W: Write, T: 't, X: Taxonomy<'t, T>>( where T: Clone + Debug + Display + Eq + Hash + PartialEq, { - let root_node = root_node.unwrap_or_else(|| taxonomy.root()); + // Defaults to root but root exists only if taxonomy is not empty. + let root_node = root_node.or(if taxonomy.is_empty() { + None + } else { + Some(taxonomy.root()) + }); + let json_data = match format { JsonFormat::NodeLink => serialize_as_node_links(taxonomy, root_node), JsonFormat::Tree => serialize_as_tree(taxonomy, root_node), @@ -332,11 +338,15 @@ where fn serialize_as_tree<'t, T: 't>( taxonomy: &'t impl Taxonomy<'t, T>, - tax_id: T, + root_node: Option, ) -> TaxonomyResult where T: Clone + Debug + Display + Eq + Hash + PartialEq, { + let tax_id = root_node.ok_or(Error::new(ErrorKind::InvalidTaxonomy( + "Taxonomy must have a root node.".to_string(), + )))?; + fn inner<'t, T: 't>(tax: &'t impl Taxonomy<'t, T>, tax_id: T) -> TaxonomyResult where T: Clone + Debug + Display + Eq + Hash + PartialEq, @@ -360,7 +370,7 @@ where fn serialize_as_node_links<'t, T: 't>( tax: &'t impl Taxonomy<'t, T>, - root_id: T, + root_node: Option, ) -> TaxonomyResult where T: Clone + Debug + Display + Eq + Hash + PartialEq, @@ -369,20 +379,22 @@ where let mut links = Vec::new(); let mut id_to_idx = HashMap::new(); - for (ix, (tid, _pre)) in tax.traverse(root_id)?.filter(|x| x.1).enumerate() { - let node = TaxNode { - id: tid.to_string(), - name: tax.name(tid.clone())?.to_string(), - rank: tax.rank(tid.clone())?, - extra: (*tax.data(tid.clone())?).clone(), - }; - nodes.push(to_value(&node).unwrap()); - id_to_idx.insert(tid.clone(), ix); - if let Some(parent_id) = tax.parent(tid)? { - links.push(json!({ - "source": ix, - "target": id_to_idx[&parent_id.0], - })); + if let Some(root_id) = root_node { + for (ix, (tid, _pre)) in tax.traverse(root_id)?.filter(|x| x.1).enumerate() { + let node = TaxNode { + id: tid.to_string(), + name: tax.name(tid.clone())?.to_string(), + rank: tax.rank(tid.clone())?, + extra: (*tax.data(tid.clone())?).clone(), + }; + nodes.push(to_value(&node).unwrap()); + id_to_idx.insert(tid.clone(), ix); + if let Some(parent_id) = tax.parent(tid)? { + links.push(json!({ + "source": ix, + "target": id_to_idx[&parent_id.0], + })); + } } } @@ -405,10 +417,20 @@ mod tests { use std::io::Cursor; #[test] - fn can_load_empty_node_link_format() { + fn can_load_and_save_empty_node_link_format() { + // Savings let example = r#"{"nodes": [], "links": []}"#; let tax = load(Cursor::new(example), None).unwrap(); assert_eq!(Taxonomy::<&str>::len(&tax), 0); + + // Loading + let mut out = Vec::new(); + save::<_, &str, _>(&mut out, &tax, JsonFormat::NodeLink, None).unwrap(); + assert_eq!( + String::from_utf8(out).unwrap(), + json!({ "nodes": [], "links": [], "directed": true, "multigraph": false, "graph": [] }) + .to_string() + ) } #[derive(Debug, Serialize, Deserialize)] @@ -445,7 +467,7 @@ mod tests { 1000 ); // Converting to Value should give us the same data - let serialized = serialize_as_node_links(&tax, Taxonomy::<&str>::root(&tax)).unwrap(); + let serialized = serialize_as_node_links(&tax, Some(Taxonomy::<&str>::root(&tax))).unwrap(); let tax2 = load_node_link_json(&serialized).unwrap(); assert_eq!(Taxonomy::<&str>::len(&tax2), 3); @@ -503,7 +525,7 @@ mod tests { 1000 ); // Converting to Value should give us the same data - let serialized = serialize_as_tree(&tax, Taxonomy::<&str>::root(&tax)).unwrap(); + let serialized = serialize_as_tree(&tax, Some(Taxonomy::<&str>::root(&tax))).unwrap(); let input_val: TaxNodeTree = from_str(&example).unwrap(); assert_eq!(serialized, to_value(&input_val).unwrap()); } diff --git a/test_python.py b/test_python.py index 1a2a1cf..9976723 100644 --- a/test_python.py +++ b/test_python.py @@ -1,3 +1,4 @@ +import json import unittest from taxonomy import Taxonomy, TaxonomyError @@ -157,6 +158,29 @@ def test_prune_works_after_editing_tree(self): pruned = tax.prune(keep=["5"]) assert pruned["5"].parent == "1" + def test_to_json_tree(self): + small_tax = self.tax.prune(remove=[str(i) for i in range(3, 12)]) + actual = json.loads(small_tax.to_json_tree().decode()) + expected = { + "id": "1", + "name": "root", + "rank": "no rank", + "children": [ + { + "id": "2", + "name": "superkingdom 1", + "rank": "superkingdom", + "children": [], + } + ], + } + self.assertEqual(actual, expected) + + def test_to_json_tree_with_empty_tree(self): + empty_tax = self.tax.prune(keep=[]) + with self.assertRaises(TaxonomyError): + empty_tax.to_json_tree() + class NewickTestCase(unittest.TestCase): def _create_tax(self): From eec31a22a85c4e58a58c8b12bc086993678ac2fe Mon Sep 17 00:00:00 2001 From: Petras Date: Fri, 10 May 2024 13:41:57 +0300 Subject: [PATCH 2/2] Empty tests for notes and links --- test_python.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/test_python.py b/test_python.py index 9976723..039b77d 100644 --- a/test_python.py +++ b/test_python.py @@ -160,7 +160,7 @@ def test_prune_works_after_editing_tree(self): def test_to_json_tree(self): small_tax = self.tax.prune(remove=[str(i) for i in range(3, 12)]) - actual = json.loads(small_tax.to_json_tree().decode()) + actual = json.loads(small_tax.to_json_tree()) expected = { "id": "1", "name": "root", @@ -181,6 +181,18 @@ def test_to_json_tree_with_empty_tree(self): with self.assertRaises(TaxonomyError): empty_tax.to_json_tree() + def test_to_json_node_links_empty_tree(self): + empty_tax = self.tax.prune(keep=[]) + actual = json.loads(empty_tax.to_json_node_links()) + expected = { + "directed": True, + "graph": [], + "links": [], + "multigraph": False, + "nodes": [], + } + self.assertEqual(actual, expected) + class NewickTestCase(unittest.TestCase): def _create_tax(self):