Redesign provenance feature: (#207)

* Remove Provenance trait
* Functions from the removed trait are now part of TableAccess.
* Reorganize the docs.

Closes #190

Author: molpopgen

src/lib.rs

@@ -426,20 +426,20 @@ pub use traits::TskitTypeAccess;
 pub use trees::{NodeTraversalOrder, Tree, TreeSequence};
 
 // Optional features
-#[cfg(any(doc, feature = "provenance"))]
+#[cfg(feature = "provenance")]
 pub mod provenance;
 
 /// A provenance ID
 ///
 /// This is an integer referring to a row of a [``provenance::ProvenanceTable``].
 ///
 /// The features for this type follow the same pattern as for [``NodeId``]
-#[cfg(any(doc, feature = "provenance"))]
+#[cfg(feature = "provenance")]
 #[repr(transparent)]
 #[derive(Copy, Clone, Debug, Eq, PartialEq, Ord, PartialOrd, std::hash::Hash)]
 pub struct ProvenanceId(tsk_id_t);
 
-#[cfg(any(doc, feature = "provenance"))]
+#[cfg(feature = "provenance")]
 impl_id_traits!(ProvenanceId);
 
 /// Handles return codes from low-level tskit functions.

src/provenance.rs

@@ -3,106 +3,21 @@
 //! This module is enabled via the `"provenance"` feature and provides
 //! the following:
 //!
-//! * trait [`Provenance`], which enables populating and accessing
-//!   [`ProvenanceTable`].
+//! * [`crate::TableCollection::add_provenance`]
+//! * [`crate::TableCollection::provenances`]
+//! * [`crate::TableCollection::provenances_iter`]
+//! * [`crate::TreeSequence::add_provenance`]
+//! * [`crate::TreeSequence::provenances`]
+//! * [`crate::TreeSequence::provenances_iter`]
+//! * [`ProvenanceTable`].
 //! * [`ProvenanceTableRow`], which is the value type returned by
 //!   [`ProvenanceTable::iter`].
 //!
-//! See [`Provenance`] for examples.
 
 use crate::bindings as ll_bindings;
 use crate::SizeType;
 use crate::{tsk_id_t, tsk_size_t, ProvenanceId, TskitError};
 
-/// Enable provenance table access.
-///
-/// `tskit` provides implementations of this trait
-/// for [`crate::TableCollection`] and [`crate::TreeSequence`].
-#[cfg_attr(
-    feature = "provenance",
-    doc = r##"
-# Examples
-
-## For table collections
-
-```
-use tskit::provenance::Provenance;
-let mut tables = tskit::TableCollection::new(1000.).unwrap();
-tables.add_provenance(&String::from("Some provenance")).unwrap();
-
-// Get reference to the table
-let prov_ref = tables.provenances();
-
-// Get the first row
-let row_0 = prov_ref.row(0).unwrap();
-
-assert_eq!(row_0.record, "Some provenance");
-
-// Get the first record
-let record_0 = prov_ref.record(0).unwrap();
-assert_eq!(record_0, row_0.record);
-
-// Get the first time stamp
-let timestamp = prov_ref.timestamp(0).unwrap();
-assert_eq!(timestamp, row_0.timestamp);
-
-// You can get the `humantime::Timestamp` object back from the `String`:
-use core::str::FromStr;
-let timestamp_string = humantime::Timestamp::from_str(&timestamp).unwrap();
-
-// Provenance transfers to the tree sequences
-let treeseq = tables.tree_sequence(tskit::TreeSequenceFlags::BUILD_INDEXES).unwrap();
-assert_eq!(treeseq.provenances().record(0).unwrap(), "Some provenance");
-// We can still compare to row_0 because it is a copy of the row data:
-assert_eq!(treeseq.provenances().record(0).unwrap(), row_0.record);
-```
-
-## For tree sequences
-
-```
-use tskit::provenance::Provenance;
-let mut tables = tskit::TableCollection::new(1000.).unwrap();
-let mut treeseq = tables.tree_sequence(tskit::TreeSequenceFlags::BUILD_INDEXES).unwrap();
-treeseq.add_provenance(&String::from("All your provenance r belong 2 us.")).unwrap();
-
-let prov_ref = treeseq.provenances();
-let row_0 = prov_ref.row(0).unwrap();
-assert_eq!(row_0.record, "All your provenance r belong 2 us.");
-let record_0 = prov_ref.record(0).unwrap();
-assert_eq!(record_0, row_0.record);
-let timestamp = prov_ref.timestamp(0).unwrap();
-assert_eq!(timestamp, row_0.timestamp);
-use core::str::FromStr;
-let dt_utc = humantime::Timestamp::from_str(&timestamp).unwrap();
-println!("utc = {}", dt_utc);
-```
-
-"##
-)]
-pub trait Provenance: crate::TableAccess {
-    /// Add provenance record with a time stamp.
-    ///
-    /// All implementation of this trait provided by `tskit` use
-    /// an `ISO 8601` format time stamp
-    /// written using the [RFC 3339](https://tools.ietf.org/html/rfc3339)
-    /// specification.
-    /// This formatting approach has been the most straightforward method
-    /// for supporting round trips to/from a [`ProvenanceTable`].
-    /// The implementations used here use the [`chrono`](https://docs.rs/chrono) crate.
-    ///
-    /// # Parameters
-    ///
-    /// * `record`: the provenance record
-    fn add_provenance(&mut self, record: &str) -> Result<ProvenanceId, TskitError>;
-    /// Return an immutable reference to the table, type [`ProvenanceTable`]
-    fn provenances(&self) -> ProvenanceTable;
-    /// Return an iterator over the rows of the [`ProvenanceTable`].
-    /// See [`ProvenanceTable::iter`] for details.
-    fn provenances_iter(&self) -> ProvenanceTableIterator {
-        crate::table_iterator::make_table_iterator::<ProvenanceTable>(self.provenances())
-    }
-}
-
 #[derive(Eq)]
 /// Row of a [`ProvenanceTable`].
 pub struct ProvenanceTableRow {
@@ -130,7 +45,7 @@ impl std::fmt::Display for ProvenanceTableRow {
     }
 }
 
-fn make_provenance_table_row(table: &ProvenanceTable, pos: tsk_id_t) -> Option<ProvenanceTableRow> {
+fn make_provenance_row(table: &ProvenanceTable, pos: tsk_id_t) -> Option<ProvenanceTableRow> {
     // panic is okay here, as we are handling a bad
     // input value before we first call this to
     // set up the iterator
@@ -153,7 +68,7 @@ impl<'a> Iterator for ProvenanceTableRefIterator<'a> {
     type Item = ProvenanceTableRow;
 
     fn next(&mut self) -> Option<Self::Item> {
-        let rv = make_provenance_table_row(self.table, self.pos);
+        let rv = make_provenance_row(self.table, self.pos);
         self.pos += 1;
         rv
     }
@@ -163,7 +78,7 @@ impl<'a> Iterator for ProvenanceTableIterator<'a> {
     type Item = ProvenanceTableRow;
 
     fn next(&mut self) -> Option<Self::Item> {
-        let rv = make_provenance_table_row(&self.table, self.pos);
+        let rv = make_provenance_row(&self.table, self.pos);
         self.pos += 1;
         rv
     }
@@ -172,7 +87,8 @@ impl<'a> Iterator for ProvenanceTableIterator<'a> {
 /// An immutable view of a provenance table.
 ///
 /// These are not created directly.
-/// Instead, use [`Provenance::provenances`]
+/// Instead, use [`crate::TableCollection::provenances`]
+/// or [`crate::TreeSequence::provenances`]
 /// to get a reference to an existing node table;
 ///
 /// # Notes
@@ -250,7 +166,7 @@ impl<'a> ProvenanceTable<'a> {
         if row.into() < 0 {
             Err(TskitError::IndexError)
         } else {
-            match make_provenance_table_row(self, row.into().0) {
+            match make_provenance_row(self, row.into().0) {
                 Some(x) => Ok(x),
                 None => Err(TskitError::IndexError),
             }
@@ -265,10 +181,10 @@ impl<'a> ProvenanceTable<'a> {
 }
 
 #[cfg(test)]
-mod test_provenance_tables {
+mod test_provenances {
     use super::*;
     use crate::test_fixtures::make_empty_table_collection;
-    use Provenance;
+    use crate::TableAccess;
 
     #[test]
     fn test_empty_record_string() {

src/table_collection.rs

@@ -769,6 +769,67 @@ impl TableCollection {
         };
         handle_tsk_return_value!(rv)
     }
+
+    #[cfg(feature = "provenance")]
+    /// Add provenance record with a time stamp.
+    ///
+    /// All implementation of this trait provided by `tskit` use
+    /// an `ISO 8601` format time stamp
+    /// written using the [RFC 3339](https://tools.ietf.org/html/rfc3339)
+    /// specification.
+    /// This formatting approach has been the most straightforward method
+    /// for supporting round trips to/from a [`crate::provenance::ProvenanceTable`].
+    /// The implementations used here use the [`humantime`](https://docs.rs/humantime/latest/humantime/) crate.
+    ///
+    /// # Parameters
+    ///
+    /// * `record`: the provenance record
+    ///
+    /// # Examples
+    /// ```
+    /// use tskit::TableAccess;
+    /// let mut tables = tskit::TableCollection::new(1000.).unwrap();
+    /// tables.add_provenance(&String::from("Some provenance")).unwrap();
+    ///
+    /// // Get reference to the table
+    /// let prov_ref = tables.provenances();
+    ///
+    /// // Get the first row
+    /// let row_0 = prov_ref.row(0).unwrap();
+    ///
+    /// assert_eq!(row_0.record, "Some provenance");
+    ///
+    /// // Get the first record
+    /// let record_0 = prov_ref.record(0).unwrap();
+    /// assert_eq!(record_0, row_0.record);
+    ///
+    /// // Get the first time stamp
+    /// let timestamp = prov_ref.timestamp(0).unwrap();
+    /// assert_eq!(timestamp, row_0.timestamp);
+    ///
+    /// // You can get the `humantime::Timestamp` object back from the `String`:
+    /// use core::str::FromStr;
+    /// let timestamp_string = humantime::Timestamp::from_str(&timestamp).unwrap();
+    ///
+    /// // Provenance transfers to the tree sequences
+    /// let treeseq = tables.tree_sequence(tskit::TreeSequenceFlags::BUILD_INDEXES).unwrap();
+    /// assert_eq!(treeseq.provenances().record(0).unwrap(), "Some provenance");
+    /// // We can still compare to row_0 because it is a copy of the row data:
+    /// assert_eq!(treeseq.provenances().record(0).unwrap(), row_0.record);
+    /// ```
+    pub fn add_provenance(&mut self, record: &str) -> Result<crate::ProvenanceId, TskitError> {
+        let timestamp = humantime::format_rfc3339(std::time::SystemTime::now()).to_string();
+        let rv = unsafe {
+            ll_bindings::tsk_provenance_table_add_row(
+                &mut (*self.inner).provenances,
+                timestamp.as_ptr() as *mut i8,
+                timestamp.len() as tsk_size_t,
+                record.as_ptr() as *mut i8,
+                record.len() as tsk_size_t,
+            )
+        };
+        handle_tsk_return_value!(rv, crate::ProvenanceId::from(rv))
+    }
 }
 
 impl TableAccess for TableCollection {
@@ -799,31 +860,15 @@ impl TableAccess for TableCollection {
     fn populations(&self) -> PopulationTable {
         PopulationTable::new_from_table(&(*self.inner).populations)
     }
-}
-
-impl crate::traits::NodeListGenerator for TableCollection {}
-
-#[cfg(any(doc, feature = "provenance"))]
-impl crate::provenance::Provenance for TableCollection {
-    fn add_provenance(&mut self, record: &str) -> Result<crate::ProvenanceId, TskitError> {
-        let timestamp = humantime::format_rfc3339(std::time::SystemTime::now()).to_string();
-        let rv = unsafe {
-            ll_bindings::tsk_provenance_table_add_row(
-                &mut (*self.inner).provenances,
-                timestamp.as_ptr() as *mut i8,
-                timestamp.len() as tsk_size_t,
-                record.as_ptr() as *mut i8,
-                record.len() as tsk_size_t,
-            )
-        };
-        handle_tsk_return_value!(rv, crate::ProvenanceId::from(rv))
-    }
 
+    #[cfg(feature = "provenance")]
     fn provenances(&self) -> crate::provenance::ProvenanceTable {
         crate::provenance::ProvenanceTable::new_from_table(&(*self.inner).provenances)
     }
 }
 
+impl crate::traits::NodeListGenerator for TableCollection {}
+
 #[cfg(test)]
 mod test {
     use super::*;

src/traits.rs

@@ -112,6 +112,20 @@ pub trait TableAccess {
     ) -> Box<dyn Iterator<Item = crate::individual_table::IndividualTableRow> + '_> {
         Box::new(make_table_iterator::<IndividualTable>(self.individuals()))
     }
+
+    #[cfg(feature = "provenance")]
+    /// Get reference to the [``ProvenanceTable``](crate::provenance::ProvenanceTable)
+    fn provenances(&self) -> crate::provenance::ProvenanceTable;
+
+    #[cfg(feature = "provenance")]
+    /// Return an iterator over provenances
+    fn provenances_iter(
+        &self,
+    ) -> Box<dyn Iterator<Item = crate::provenance::ProvenanceTableRow> + '_> {
+        Box::new(crate::table_iterator::make_table_iterator::<
+            crate::provenance::ProvenanceTable,
+        >(self.provenances()))
+    }
 }
 
 /// Interface for returning lists of node ids from