fix: update generic bounds on metadata getters (#370)

* update metadata fn docs

BREAKING CHANGE: previous code that jumped through hoops
to record metadata type A and retrieve type B, where
A and B are metadata type trait impls for different table
types, will no fail to compile.

Author: molpopgen

src/edge_table.rs

@@ -136,7 +136,23 @@ impl<'a> EdgeTable<'a> {
         unsafe_tsk_column_access_and_map_into!(row.into().0, 0, self.num_rows(), self.table_, right)
     }
 
-    pub fn metadata<T: metadata::MetadataRoundtrip>(
+    /// Retrieve decoded metadata for a `row`.
+    ///
+    /// # Returns
+    ///
+    /// * `Some(Ok(T))` if `row` is valid and decoding succeeded.
+    /// * `Some(Err(_))` if `row` is not valid and decoding failed.
+    /// * `None` if `row` is not valid.
+    ///
+    /// # Errors
+    ///
+    /// * [`TskitError::MetadataError`] if decoding fails.
+    ///
+    /// # Examples.
+    ///
+    /// The big-picture semantics are the same for all table types.
+    /// See [`crate::IndividualTable::metadata`] for examples.
+    pub fn metadata<T: metadata::EdgeMetadata>(
         &'a self,
         row: EdgeId,
     ) -> Option<Result<T, TskitError>> {

src/individual_table.rs

@@ -152,14 +152,13 @@ impl<'a> IndividualTable<'a> {
     ///
     /// # Returns
     ///
-    /// The result type is `Option<T>`
-    /// where `T`: [`tskit::metadata::IndividualMetadata`](crate::metadata::IndividualMetadata).
-    /// `Some(T)` if there is metadata.  `None` if the metadata field is empty for a given
-    /// row.
+    /// * `Some(Ok(T))` if `row` is valid and decoding succeeded.
+    /// * `Some(Err(_))` if `row` is not valid and decoding failed.
+    /// * `None` if `row` is not valid.
     ///
     /// # Errors
     ///
-    /// * [`TskitError::IndexError`] if `row` is out of range.
+    /// * [`TskitError::MetadataError`] if decoding fails.
     ///
     /// # Examples
     ///
@@ -190,15 +189,16 @@ impl<'a> IndividualTable<'a> {
     /// # let metadata = IndividualMetadata{x: 1};
     /// # assert!(tables.add_individual_with_metadata(0, None, None,
     /// #                                             &metadata).is_ok());
-    /// // We know the metadata are here, so we unwrap the Result and the Option
+    /// // We know the metadata are here, so we unwrap the Option and the Result:
     /// let decoded = tables.individuals().metadata::<IndividualMetadata>(0.into()).unwrap().unwrap();
     /// assert_eq!(decoded.x, 1);
     /// # }
     /// ```
     ///
     /// ## Checking for errors and absence of metadata
     ///
-    /// Handling both the possibility of error and optional metadata leads to some verbosity:
+    /// The `Option<Result<_>>` return value allows all
+    /// three return possibilities to be easily covered:
     ///
     /// ```
     /// # #[cfg(feature = "derive")] {
@@ -213,22 +213,112 @@ impl<'a> IndividualTable<'a> {
     /// # assert!(tables
     /// #     .add_individual_with_metadata(0, None, None, &metadata)
     /// #     .is_ok());
-    /// // First, check the Result.
-    /// let decoded_option = match tables
-    ///     .individuals()
-    ///     .metadata::<IndividualMetadata>(0.into())
+    /// match tables.individuals().metadata::<IndividualMetadata>(0.into())
     /// {
-    ///     Some(metadata_option) => metadata_option,
-    ///     None => panic!("expected metadata"),
+    ///     Some(Ok(metadata)) => assert_eq!(metadata.x, 1),
+    ///     Some(Err(_)) => panic!("got an error??"),
+    ///     None => panic!("Got None??"),
     /// };
-    /// // Now, check the contents of the Option
-    /// match decoded_option {
-    ///     Ok(metadata) => assert_eq!(metadata.x, 1),
-    ///     Err(e) => panic!("error decoding metadata: {:?}", e),
+    /// # }
+    /// ```
+    ///
+    /// ## Attempting to use the wrong type.
+    ///
+    /// Let's define a mutation metadata type with the exact same fields
+    /// as our individual metadata defined above:
+    ///
+    /// ```
+    /// # #[cfg(feature = "derive")] {
+    /// #[derive(serde::Serialize, serde::Deserialize, tskit::metadata::MutationMetadata)]
+    /// #[serializer("serde_json")]
+    /// struct MutationMetadata {
+    ///    x: i32,
+    /// }
+    /// # }
+    /// ```
+    ///
+    /// This type has the wrong trait bound and will cause compilation to fail:
+    ///
+    #[cfg_attr(
+        feature = "derive",
+        doc = r##"
+```compile_fail
+# #[derive(serde::Serialize, serde::Deserialize, tskit::metadata::MutationMetadata)]
+# #[serializer("serde_json")]
+# struct MutationMetadata {
+#    x: i32,
+# }
+# use tskit::TableAccess;
+# let mut tables = tskit::TableCollection::new(10.).unwrap();
+match tables.individuals().metadata::<MutationMetadata>(0.into())
+{
+    Some(Ok(metadata)) => assert_eq!(metadata.x, 1),
+    Some(Err(_)) => panic!("got an error??"),
+    None => panic!("Got None??"),
+};
+```
+"##
+    )]
+    ///
+    /// ## Limitations: different type, same trait bound
+    ///
+    /// Finally, let us consider a different struct that has identical
+    /// fields to `IndividualMetadata` defined above and also implements
+    /// the correct trait:
+    ///
+    /// ```
+    /// # #[cfg(feature = "derive")] {
+    /// #[derive(serde::Serialize, serde::Deserialize, tskit::metadata::IndividualMetadata)]
+    /// #[serializer("serde_json")]
+    /// struct IndividualMetadataToo {
+    ///    x: i32,
     /// }
     /// # }
     /// ```
-    pub fn metadata<T: metadata::MetadataRoundtrip>(
+    ///
+    /// Let's walk through a detailed example:
+    ///
+    /// ```
+    /// # #[cfg(feature = "derive")] {
+    /// # use tskit::TableAccess;
+    /// # #[derive(serde::Serialize, serde::Deserialize, tskit::metadata::IndividualMetadata)]
+    /// # #[serializer("serde_json")]
+    /// # struct IndividualMetadata {
+    /// #     x: i32,
+    /// # }
+    /// # #[derive(serde::Serialize, serde::Deserialize, tskit::metadata::IndividualMetadata)]
+    /// # #[serializer("serde_json")]
+    /// # struct IndividualMetadataToo {
+    /// #    x: i32,
+    /// # }
+    /// // create a mutable table collection
+    /// let mut tables = tskit::TableCollection::new(100.).unwrap();
+    /// // Create some metadata based on our FIRST type
+    /// let metadata = IndividualMetadata { x: 1 };
+    /// // Add a row with our metadata
+    /// assert!(tables.add_individual_with_metadata(0, None, None, &metadata).is_ok());
+    /// // Trying to fetch using our SECOND type as the generic type works!
+    /// match tables.individuals().metadata::<IndividualMetadataToo>(0.into())
+    /// {
+    ///     Some(Ok(metadata)) => assert_eq!(metadata.x, 1),
+    ///     Some(Err(_)) => panic!("got an error??"),
+    ///     None => panic!("Got None??"),
+    /// };
+    /// # }
+    /// ```
+    ///
+    /// What is going on here?
+    /// Both types satisfy the same trait bound ([`metadata::IndividualMetadata`])
+    /// and their data fields look identical to `serde_json`.
+    /// Thus, one is exchangeable for the other because they have the exact same
+    /// *behavior*.
+    ///
+    /// However, it is also true that this is (often/usually/always) not exactly what we want.
+    /// We are experimenting with encapsulation APIs involving traits with
+    /// [associated
+    /// types](https://doc.rust-lang.org/book/ch19-03-advanced-traits.html#specifying-placeholder-types-in-trait-definitions-with-associated-types) to enforce at *compile time* that exactly one type (`struct/enum`, etc.) is a valid
+    /// metadata type for a table.
+    pub fn metadata<T: metadata::IndividualMetadata>(
         &'a self,
         row: IndividualId,
     ) -> Option<Result<T, TskitError>> {

src/migration_table.rs

@@ -165,14 +165,23 @@ impl<'a> MigrationTable<'a> {
         unsafe_tsk_column_access_and_map_into!(row.into().0, 0, self.num_rows(), self.table_, time)
     }
 
-    /// Return the metadata for a given row.
+    /// Retrieve decoded metadata for a `row`.
     ///
     /// # Returns
     ///
-    /// * `Some(Ok(metadata))` if `row` is valid and decoding succeeded
-    /// * `Some(Err(_))` if `row` is valid and decoding failed.
+    /// * `Some(Ok(T))` if `row` is valid and decoding succeeded.
+    /// * `Some(Err(_))` if `row` is not valid and decoding failed.
     /// * `None` if `row` is not valid.
-    pub fn metadata<T: metadata::MetadataRoundtrip>(
+    ///
+    /// # Errors
+    ///
+    /// * [`TskitError::MetadataError`] if decoding fails.
+    ///
+    /// # Examples.
+    ///
+    /// The big-picture semantics are the same for all table types.
+    /// See [`crate::IndividualTable::metadata`] for examples.
+    pub fn metadata<T: metadata::MigrationMetadata>(
         &'a self,
         row: MigrationId,
     ) -> Option<Result<T, TskitError>> {

src/mutation_table.rs

@@ -161,7 +161,23 @@ impl<'a> MutationTable<'a> {
         )
     }
 
-    pub fn metadata<T: metadata::MetadataRoundtrip>(
+    /// Retrieve decoded metadata for a `row`.
+    ///
+    /// # Returns
+    ///
+    /// * `Some(Ok(T))` if `row` is valid and decoding succeeded.
+    /// * `Some(Err(_))` if `row` is not valid and decoding failed.
+    /// * `None` if `row` is not valid.
+    ///
+    /// # Errors
+    ///
+    /// * [`TskitError::MetadataError`] if decoding fails.
+    ///
+    /// # Examples.
+    ///
+    /// The big-picture semantics are the same for all table types.
+    /// See [`crate::IndividualTable::metadata`] for examples.
+    pub fn metadata<T: metadata::MutationMetadata>(
         &'a self,
         row: MutationId,
     ) -> Option<Result<T, TskitError>> {

src/node_table.rs

@@ -327,7 +327,23 @@ impl<'a> NodeTable<'a> {
         )
     }
 
-    pub fn metadata<T: metadata::MetadataRoundtrip>(
+    /// Retrieve decoded metadata for a `row`.
+    ///
+    /// # Returns
+    ///
+    /// * `Some(Ok(T))` if `row` is valid and decoding succeeded.
+    /// * `Some(Err(_))` if `row` is not valid and decoding failed.
+    /// * `None` if `row` is not valid.
+    ///
+    /// # Errors
+    ///
+    /// * [`TskitError::MetadataError`] if decoding fails.
+    ///
+    /// # Examples.
+    ///
+    /// The big-picture semantics are the same for all table types.
+    /// See [`crate::IndividualTable::metadata`] for examples.
+    pub fn metadata<T: metadata::NodeMetadata>(
         &'a self,
         row: NodeId,
     ) -> Option<Result<T, TskitError>> {

src/population_table.rs

@@ -80,7 +80,23 @@ impl<'a> PopulationTable<'a> {
         self.table_.num_rows.into()
     }
 
-    pub fn metadata<T: metadata::MetadataRoundtrip>(
+    /// Retrieve decoded metadata for a `row`.
+    ///
+    /// # Returns
+    ///
+    /// * `Some(Ok(T))` if `row` is valid and decoding succeeded.
+    /// * `Some(Err(_))` if `row` is not valid and decoding failed.
+    /// * `None` if `row` is not valid.
+    ///
+    /// # Errors
+    ///
+    /// * [`TskitError::MetadataError`] if decoding fails.
+    ///
+    /// # Examples.
+    ///
+    /// The big-picture semantics are the same for all table types.
+    /// See [`crate::IndividualTable::metadata`] for examples.
+    pub fn metadata<T: metadata::PopulationMetadata>(
         &'a self,
         row: PopulationId,
     ) -> Option<Result<T, TskitError>> {

src/site_table.rs

@@ -112,7 +112,23 @@ impl<'a> SiteTable<'a> {
         )
     }
 
-    pub fn metadata<T: metadata::MetadataRoundtrip>(
+    /// Retrieve decoded metadata for a `row`.
+    ///
+    /// # Returns
+    ///
+    /// * `Some(Ok(T))` if `row` is valid and decoding succeeded.
+    /// * `Some(Err(_))` if `row` is not valid and decoding failed.
+    /// * `None` if `row` is not valid.
+    ///
+    /// # Errors
+    ///
+    /// * [`TskitError::MetadataError`] if decoding fails.
+    ///
+    /// # Examples.
+    ///
+    /// The big-picture semantics are the same for all table types.
+    /// See [`crate::IndividualTable::metadata`] for examples.
+    pub fn metadata<T: metadata::SiteMetadata>(
         &'a self,
         row: SiteId,
     ) -> Option<Result<T, TskitError>> {