Repository: biocore/improved-octo-waddle (language: python)

Issue:
raise if empty tree on shear and collapse



Discussion / hints:

Fix patch:
diff --git a/bp/_bp.pyx b/bp/_bp.pyx
index 19aa0b7..10ebec6 100644
--- a/bp/_bp.pyx
+++ b/bp/_bp.pyx
@@ -224,9 +224,7 @@ cdef class BP:
             self._edges = np.full(self.B.size, 0, dtype=INT32)
             self._edge_lookup = None
 
-        # construct a rank index. These operations are performed frequently,
-        # and easy to cache at a relatively minor memory expense
-        #TODO: leverage rmm tree, and calculate rank on the fly
+        # precursor for select index cache
         _r_index_0 = np.cumsum((1 - B), dtype=SIZE)
         _r_index_1 = np.cumsum(B, dtype=SIZE)
 
@@ -784,7 +782,7 @@ cdef class BP:
         """
         cdef:
             SIZE_t i, n = len(tips)
-            SIZE_t p, t
+            SIZE_t p, t, count = 0
             BIT_ARRAY* mask
             BP new_bp
 
@@ -797,6 +795,7 @@ cdef class BP:
             if self.isleaf(i):
                 if self.name(i) in tips:  # gil is required for set operation
                     with nogil:
+                        count += 1
                         bit_array_set_bit(mask, i)
                         bit_array_set_bit(mask, i + 1)
 
@@ -807,6 +806,10 @@ cdef class BP:
 
                             p = self.parent(p)
 
+        if count == 0:
+            bit_array_free(mask)
+            raise ValueError("No requested tips found")
+                
         new_bp = self._mask_from_self(mask, self._lengths)
         bit_array_free(mask)
         return new_bp
@@ -825,6 +828,7 @@ cdef class BP:
 
         n = bit_array_length(mask)
         mask_sum = bit_array_num_bits_set(mask)
+
         k = 0
 
         lengths_ptr = &lengths[0]
diff --git a/bp/_io.pyx b/bp/_io.pyx
index a90d1cc..d6c2746 100644
--- a/bp/_io.pyx
+++ b/bp/_io.pyx
@@ -142,6 +142,12 @@ cpdef parse_newick(unicode data):
         np.ndarray[np.double_t, ndim=1] lengths
         np.ndarray[np.int32_t, ndim=1] edges
 
+    if data.count(',') == 0:
+        raise ValueError("Only trees with more than 1 node supported")
+
+    if not data.endswith(';'):
+        raise ValueError("Newick does not appear terminated with a semicolon")
+
     datalen = len(data)
     topology = _newick_to_bp(data)
 


Test patch:
diff --git a/bp/tests/test_bp.py b/bp/tests/test_bp.py
index a0b89e8..f344a90 100644
--- a/bp/tests/test_bp.py
+++ b/bp/tests/test_bp.py
@@ -264,6 +264,14 @@ def test_shear(self):
         obs = self.BP.shear(in_).B
         npt.assert_equal(obs, exp)
 
+    def test_shear_raise_tree_is_empty(self):
+        names = np.array(['r', '2', '3', None, '4', None, '5', '6', None, None, None, '7', None, '8', '9', '10', None,
+                          '11', None, None, None, None])
+        lengths = np.array([0, 1, 2, 0, 3, 0, 4, 5, 0, 0, 0, 6, 0, 7, 8, 9, 0, 10, 0, 0, 0, 0], dtype=np.double)
+        self.BP.set_names(names)
+        with self.assertRaises(ValueError):
+            self.BP.shear({'not', 'in', 'tree'})
+
     def test_collapse(self):
         names = np.array(['r', '2', '3', None, '4', None, '5', '6', None, None, None, '7', None, '8', '9', '10', None,
                           '11', None, None, None, None])
diff --git a/bp/tests/test_io.py b/bp/tests/test_io.py
index 21be077..7a50ebf 100644
--- a/bp/tests/test_io.py
+++ b/bp/tests/test_io.py
@@ -55,6 +55,21 @@ def test_write_newick_cases(self):
             obs = buf.read()
             self._compare_newick(obs, test)
 
+    def test_parse_newick_singlenode_bug(self):
+        # https://github.com/wasade/improved-octo-waddle/issues/29
+        test = 'i:1;'
+
+        # let's not allow this edge case
+        with self.assertRaises(ValueError):
+            parse_newick(test)
+
+    def test_parse_newick_no_semicolon_bug(self):
+        # https://github.com/wasade/improved-octo-waddle/issues/26
+        test = "((h:1, i:1, j:1, k:1, l: 1),(e:1,f:1),(n:1,o:1,p:1))a:1"
+
+        with self.assertRaises(ValueError):
+            parse_newick(test)
+
     def test_write_newick_underscore_bug(self):
         test = "(((a)b)'c_foo',((d)e)f)r;"
         buf = io.StringIO()