Deprecate Tree.num_nodes

Fixes tskit-dev#1966
hyanwong · Nov 30, 2021 · 87b11a8 · 87b11a8
1 parent 1e15ea0
commit 87b11a8
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 20 deletions.
diff --git a/python/CHANGELOG.rst b/python/CHANGELOG.rst
@@ -4,6 +4,10 @@
 
 **Breaking changes**
 
+- The ``Tree.num_nodes`` method is now deprecated with a warning, because it confusingly
+  returns the number of nodes in the entire tree sequence, rather than in the tree.
+  (:user:`hyanwong`, :issue:`1966` :pr:`1968`)
+
 - The CLI ``info`` command now gives more detailed information on the tree sequence
   (:user:`benjeffery`, :pr:`1611`)
 

diff --git a/python/_tskitmodule.c b/python/_tskitmodule.c
@@ -9486,19 +9486,6 @@ Tree_get_sample_size(Tree *self)
     return ret;
 }
 
-static PyObject *
-Tree_get_num_nodes(Tree *self)
-{
-    PyObject *ret = NULL;
-
-    if (Tree_check_state(self) != 0) {
-        goto out;
-    }
-    ret = Py_BuildValue("n", (Py_ssize_t) self->tree->num_nodes);
-out:
-    return ret;
-}
-
 static PyObject *
 Tree_get_num_roots(Tree *self)
 {
@@ -10478,10 +10465,6 @@ static PyMethodDef Tree_methods[] = {
         .ml_meth = (PyCFunction) Tree_get_sample_size,
         .ml_flags = METH_NOARGS,
         .ml_doc = "Returns the number of samples in this tree." },
-    { .ml_name = "get_num_nodes",
-        .ml_meth = (PyCFunction) Tree_get_num_nodes,
-        .ml_flags = METH_NOARGS,
-        .ml_doc = "Returns the number of nodes in this tree." },
     { .ml_name = "get_num_roots",
         .ml_meth = (PyCFunction) Tree_get_num_roots,
         .ml_flags = METH_NOARGS,

diff --git a/python/tskit/trees.py b/python/tskit/trees.py
@@ -1403,12 +1403,22 @@ def is_descendant(self, u, v):
     def num_nodes(self):
         """
         Returns the number of nodes in the :class:`TreeSequence` this tree is in.
-        Equivalent to ``tree.tree_sequence.num_nodes``. To find the number of
-        nodes that are reachable from all roots use ``len(list(tree.nodes()))``.
+        Equivalent to ``tree.tree_sequence.num_nodes``.
+
+        .. deprecated:: 0.4
+            Use :attr:`Tree.tree_sequence.num_nodes` if you want the number of nodes
+            in the entire tree sequence, or ``len(list(tree.nodes()))`` to find the
+            number of nodes that are reachable from all roots in this tree.
 
         :rtype: int
+
         """
-        return self._ll_tree.get_num_nodes()
+        warnings.warn(
+            "This property is a deprecated alias for Tree.tree_sequence.num_nodes"
+            "and will be removed in the future",
+            FutureWarning,
+        )
+        return self.tree_sequence.num_nodes
 
     @property
     def num_roots(self):