From 94957617342912095e5da6e0907fd27667350c3f Mon Sep 17 00:00:00 2001 From: molpopogen Date: Sat, 11 Dec 2021 14:22:28 -0800 Subject: [PATCH] Add tskit::SizeType * tsk_id_t and tsk_size_t removed from prelude. * tsk_id_t and tsk_size_t no longer publicly exported from lib.rs * Public API now returns SizeType instead of tsk_size_t Closes #181 --- src/_macros.rs | 55 ++++++++++++++++--- src/edge_table.rs | 19 +++++-- src/error.rs | 3 ++ src/individual_table.rs | 17 ++++-- src/lib.rs | 115 ++++++++++++++++++++++++++++++++++++++-- src/metadata.rs | 13 ++--- src/migration_table.rs | 16 ++++-- src/mutation_table.rs | 20 +++++-- src/node_table.rs | 18 +++++-- src/population_table.rs | 20 +++++-- src/prelude.rs | 4 +- src/provenance.rs | 14 +++-- src/site_table.rs | 18 +++++-- src/table_collection.rs | 49 ++++++++++------- src/traits.rs | 2 +- src/trees.rs | 13 ++--- 16 files changed, 316 insertions(+), 80 deletions(-) diff --git a/src/_macros.rs b/src/_macros.rs index 6f5f33067..aa4f06632 100644 --- a/src/_macros.rs +++ b/src/_macros.rs @@ -46,13 +46,15 @@ macro_rules! unsafe_tsk_column_access { macro_rules! unsafe_tsk_ragged_column_access { ($i: expr, $lo: expr, $hi: expr, $array: expr, $offset_array: expr, $offset_array_len: expr) => {{ - if $i < $lo || $i >= ($hi as tsk_id_t) { + use std::convert::TryFrom; + let i = crate::SizeType::try_from($i)?; + if $i < $lo || i >= $hi { Err(TskitError::IndexError {}) } else if $offset_array_len == 0 { Ok(None) } else { let start = unsafe { *$offset_array.offset($i as isize) }; - let stop = if $i < ($hi as tsk_id_t) { + let stop = if i < $hi { unsafe { *$offset_array.offset(($i + 1) as isize) } } else { $offset_array_len as tsk_size_t @@ -70,13 +72,15 @@ macro_rules! unsafe_tsk_ragged_column_access { }}; ($i: expr, $lo: expr, $hi: expr, $array: expr, $offset_array: expr, $offset_array_len: expr, $output_id_type: expr) => {{ - if $i < $lo || $i >= ($hi as tsk_id_t) { + use std::convert::TryFrom; + let i = crate::SizeType::try_from($i)?; + if $i < $lo || i >= $hi { Err(TskitError::IndexError {}) } else if $offset_array_len == 0 { Ok(None) } else { let start = unsafe { *$offset_array.offset($i as isize) }; - let stop = if $i < ($hi as tsk_id_t) { + let stop = if i < $hi { unsafe { *$offset_array.offset(($i + 1) as isize) } } else { $offset_array_len as tsk_size_t @@ -99,13 +103,15 @@ macro_rules! unsafe_tsk_ragged_column_access { #[allow(unused_macros)] macro_rules! unsafe_tsk_ragged_char_column_access { ($i: expr, $lo: expr, $hi: expr, $array: expr, $offset_array: expr, $offset_array_len: expr) => {{ - if $i < $lo || $i >= ($hi as tsk_id_t) { + use std::convert::TryFrom; + let i = crate::SizeType::try_from($i)?; + if $i < $lo || i >= $hi { Err(TskitError::IndexError {}) } else if $offset_array_len == 0 { Ok(None) } else { let start = unsafe { *$offset_array.offset($i as isize) }; - let stop = if $i < ($hi as tsk_id_t) { + let stop = if i < $hi { unsafe { *$offset_array.offset(($i + 1) as isize) } } else { $offset_array_len as tsk_size_t @@ -299,6 +305,14 @@ macro_rules! impl_id_traits { } } + impl std::convert::TryFrom<$idtype> for crate::SizeType { + type Error = crate::TskitError; + + fn try_from(value: $idtype) -> Result { + crate::SizeType::try_from(value.0) + } + } + impl PartialEq<$crate::tsk_id_t> for $idtype { fn eq(&self, other: &$crate::tsk_id_t) -> bool { self.0 == *other @@ -325,6 +339,35 @@ macro_rules! impl_id_traits { }; } +macro_rules! impl_size_type_comparisons_for_row_ids { + ($idtype: ty) => { + impl PartialEq<$idtype> for crate::SizeType { + fn eq(&self, other: &$idtype) -> bool { + self.0 == other.0 as crate::bindings::tsk_size_t + } + } + + impl PartialEq for $idtype { + fn eq(&self, other: &crate::SizeType) -> bool { + (self.0 as crate::bindings::tsk_size_t) == other.0 + } + } + + impl PartialOrd<$idtype> for crate::SizeType { + fn partial_cmp(&self, other: &$idtype) -> Option { + self.0 + .partial_cmp(&(other.0 as crate::bindings::tsk_size_t)) + } + } + + impl PartialOrd for $idtype { + fn partial_cmp(&self, other: &crate::SizeType) -> Option { + (self.0 as crate::bindings::tsk_size_t).partial_cmp(&other.0) + } + } + }; +} + /// Convenience macro to handle implementing /// [`crate::metadata::MetadataRoundtrip`] #[macro_export] diff --git a/src/edge_table.rs b/src/edge_table.rs index 30ac3d5b9..bea314d04 100644 --- a/src/edge_table.rs +++ b/src/edge_table.rs @@ -1,6 +1,6 @@ use crate::bindings as ll_bindings; use crate::metadata; -use crate::{tsk_id_t, tsk_size_t, TskitError}; +use crate::{tsk_id_t, TskitError}; use crate::{EdgeId, NodeId}; /// Row of an [`EdgeTable`] @@ -25,7 +25,12 @@ impl PartialEq for EdgeTableRow { } fn make_edge_table_row(table: &EdgeTable, pos: tsk_id_t) -> Option { - if pos < table.num_rows() as tsk_id_t { + use std::convert::TryFrom; + // panic is okay here, as we are handling a bad + // input value before we first call this to + // set up the iterator + let p = crate::SizeType::try_from(pos).unwrap(); + if p < table.num_rows() { let rv = EdgeTableRow { id: pos.into(), left: table.left(pos).unwrap(), @@ -78,8 +83,8 @@ impl<'a> EdgeTable<'a> { } /// Return the number of rows - pub fn num_rows(&'a self) -> tsk_size_t { - self.table_.num_rows + pub fn num_rows(&'a self) -> crate::SizeType { + self.table_.num_rows.into() } /// Return the ``parent`` value from row ``row`` of the table. @@ -147,6 +152,10 @@ impl<'a> EdgeTable<'a> { /// /// [`TskitError::IndexError`] if `r` is out of range. pub fn row + Copy>(&self, r: E) -> Result { - table_row_access!(r.into().0, self, make_edge_table_row) + let ri = r.into(); + if ri < 0 { + return Err(crate::TskitError::IndexError); + } + table_row_access!(ri.0, self, make_edge_table_row) } } diff --git a/src/error.rs b/src/error.rs index c16573403..603c26bb4 100644 --- a/src/error.rs +++ b/src/error.rs @@ -5,6 +5,9 @@ use thiserror::Error; #[derive(Error, Debug)] pub enum TskitError { + /// Returned when conversion attempts fail + #[error("range error: {}", *.0)] + RangeError(&'static str), /// Used when bad input is encountered. #[error("we received {} but expected {}",*got, *expected)] ValueError { got: String, expected: String }, diff --git a/src/individual_table.rs b/src/individual_table.rs index 1618af70c..ba8895bac 100644 --- a/src/individual_table.rs +++ b/src/individual_table.rs @@ -49,7 +49,12 @@ pub struct IndividualTable<'a> { } fn make_individual_table_row(table: &IndividualTable, pos: tsk_id_t) -> Option { - if pos < table.num_rows() as tsk_id_t { + use std::convert::TryFrom; + // panic is okay here, as we are handling a bad + // input value before we first call this to + // set up the iterator + let p = crate::SizeType::try_from(pos).unwrap(); + if p < table.num_rows() { let rv = IndividualTableRow { id: pos.into(), flags: table.flags(pos).unwrap(), @@ -96,8 +101,8 @@ impl<'a> IndividualTable<'a> { } /// Return the number of rows - pub fn num_rows(&'a self) -> ll_bindings::tsk_size_t { - self.table_.num_rows + pub fn num_rows(&'a self) -> crate::SizeType { + self.table_.num_rows.into() } /// Return the flags for a given row. @@ -181,6 +186,10 @@ impl<'a> IndividualTable<'a> { &self, r: I, ) -> Result { - table_row_access!(r.into().0, self, make_individual_table_row) + let ri = r.into(); + if ri < 0 { + return Err(crate::TskitError::IndexError); + } + table_row_access!(ri.0, self, make_individual_table_row) } } diff --git a/src/lib.rs b/src/lib.rs index 939d75c76..ab8d23635 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -102,8 +102,9 @@ pub use bindings::TSK_NODE_IS_SAMPLE; // re-export types, too pub use bindings::tsk_flags_t; -pub use bindings::tsk_id_t; -pub use bindings::tsk_size_t; + +use bindings::tsk_id_t; +use bindings::tsk_size_t; /// A node ID /// @@ -116,7 +117,7 @@ pub use bindings::tsk_size_t; /// /// ``` /// use tskit::NodeId; -/// use tskit::tsk_id_t; +/// use tskit::bindings::tsk_id_t; /// /// let x: tsk_id_t = 1; /// let y: NodeId = NodeId::from(x); @@ -137,7 +138,7 @@ pub use bindings::tsk_size_t; /// /// ``` /// use tskit::NodeId; -/// use tskit::tsk_id_t; +/// use tskit::bindings::tsk_id_t; /// /// fn interesting>(x: N) -> NodeId { /// x.into() @@ -226,6 +227,112 @@ impl_id_traits!(MutationId); impl_id_traits!(MigrationId); impl_id_traits!(EdgeId); +impl_size_type_comparisons_for_row_ids!(NodeId); +impl_size_type_comparisons_for_row_ids!(EdgeId); +impl_size_type_comparisons_for_row_ids!(SiteId); +impl_size_type_comparisons_for_row_ids!(MutationId); +impl_size_type_comparisons_for_row_ids!(PopulationId); +impl_size_type_comparisons_for_row_ids!(MigrationId); + +/// Wraps `tsk_size_t` +/// +/// This type plays the role of C's `size_t` in the `tskit` C library. +/// +/// # Examples +/// +/// ``` +/// let s = tskit::SizeType::from(1 as tskit::bindings::tsk_size_t); +/// let mut t: tskit::bindings::tsk_size_t = s.into(); +/// assert!(t == s); +/// assert!(t == 1); +/// let u = tskit::SizeType::from(s); +/// assert!(u == s); +/// t += 1; +/// assert!(t > s); +/// assert!(s < t); +/// ``` +/// +/// #[repr(transparent)] +#[derive(Copy, Clone, Debug, Eq, PartialEq, Ord, PartialOrd, std::hash::Hash)] +pub struct SizeType(tsk_size_t); + +impl std::fmt::Display for SizeType { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "SizeType({})", self.0) + } +} + +impl From for SizeType { + fn from(value: tsk_size_t) -> Self { + Self(value) + } +} + +impl From for tsk_size_t { + fn from(value: SizeType) -> Self { + value.0 + } +} + +impl From for usize { + fn from(value: SizeType) -> Self { + value.0 as usize + } +} + +impl From for SizeType { + fn from(value: usize) -> Self { + Self(value as tsk_size_t) + } +} + +impl std::convert::TryFrom for SizeType { + type Error = crate::TskitError; + + fn try_from(value: tsk_id_t) -> Result { + match value >= 0 { + true => Ok(Self(value as crate::bindings::tsk_size_t)), + false => Err(crate::TskitError::RangeError(stringify!(value.0))), + } + } +} + +impl std::convert::TryFrom for tsk_id_t { + type Error = crate::TskitError; + + fn try_from(value: SizeType) -> Result { + if value.0 > tsk_id_t::MAX as tsk_size_t { + Err(TskitError::RangeError(stringify!(value.0))) + } else { + Ok(value.0 as tsk_id_t) + } + } +} + +impl PartialEq for tsk_size_t { + fn eq(&self, other: &SizeType) -> bool { + *self == other.0 + } +} + +impl PartialEq for SizeType { + fn eq(&self, other: &tsk_size_t) -> bool { + self.0 == *other + } +} + +impl PartialOrd for SizeType { + fn partial_cmp(&self, other: &tsk_size_t) -> Option { + self.0.partial_cmp(other) + } +} + +impl PartialOrd for tsk_size_t { + fn partial_cmp(&self, other: &SizeType) -> Option { + self.partial_cmp(&other.0) + } +} + // tskit defines this via a type cast // in a macro. bindgen thus misses it. // See bindgen issue 316. diff --git a/src/metadata.rs b/src/metadata.rs index 1eeda527f..e0a435787 100644 --- a/src/metadata.rs +++ b/src/metadata.rs @@ -164,6 +164,7 @@ //! into `Python` via the `tskit` `Python API`. use crate::bindings::{tsk_id_t, tsk_size_t}; +use crate::SizeType; use thiserror::Error; #[cfg(feature = "derive")] @@ -233,8 +234,8 @@ impl EncodedMetadata { } } - pub(crate) fn len(&self) -> tsk_size_t { - self.encoded.len() as tsk_size_t + pub(crate) fn len(&self) -> SizeType { + self.encoded.len().into() } } @@ -344,8 +345,8 @@ mod tests { let enc = EncodedMetadata::new(&f).unwrap(); let p = enc.as_ptr(); let mut d = vec![]; - for i in 0..enc.len() { - d.push(unsafe { *p.add(i as usize) as u8 }); + for i in 0..usize::from(enc.len()) { + d.push(unsafe { *p.add(i) as u8 }); } let df = F::decode(&d).unwrap(); assert_eq!(f.x, df.x); @@ -378,8 +379,8 @@ mod test_serde { let enc = EncodedMetadata::new(&f).unwrap(); let p = enc.as_ptr(); let mut d = vec![]; - for i in 0..enc.len() { - d.push(unsafe { *p.add(i as usize) as u8 }); + for i in 0..usize::from(enc.len()) { + d.push(unsafe { *p.add(i) as u8 }); } let df = F::decode(&d).unwrap(); assert_eq!(f.x, df.x); diff --git a/src/migration_table.rs b/src/migration_table.rs index eeb2612d7..4de58b7a1 100644 --- a/src/migration_table.rs +++ b/src/migration_table.rs @@ -1,5 +1,6 @@ use crate::bindings as ll_bindings; use crate::metadata; +use crate::SizeType; use crate::{tsk_id_t, TskitError}; use crate::{MigrationId, NodeId, PopulationId}; @@ -29,7 +30,12 @@ impl PartialEq for MigrationTableRow { } fn make_migration_table_row(table: &MigrationTable, pos: tsk_id_t) -> Option { - if pos < table.num_rows() as tsk_id_t { + use std::convert::TryFrom; + // panic is okay here, as we are handling a bad + // input value before we first call this to + // set up the iterator + let p = crate::SizeType::try_from(pos).unwrap(); + if p < table.num_rows() { Some(MigrationTableRow { id: pos.into(), left: table.left(pos).unwrap(), @@ -85,8 +91,8 @@ impl<'a> MigrationTable<'a> { } /// Return the number of rows - pub fn num_rows(&'a self) -> ll_bindings::tsk_size_t { - self.table_.num_rows + pub fn num_rows(&'a self) -> SizeType { + self.table_.num_rows.into() } /// Return the left coordinate for a given row. @@ -187,6 +193,10 @@ impl<'a> MigrationTable<'a> { /// /// [`TskitError::IndexError`] if `r` is out of range. pub fn row + Copy>(&self, r: M) -> Result { + let ri = r.into(); + if ri < 0 { + return Err(crate::TskitError::IndexError); + } table_row_access!(r.into().0, self, make_migration_table_row) } } diff --git a/src/mutation_table.rs b/src/mutation_table.rs index 4a8cf21ed..1c30ee4f8 100644 --- a/src/mutation_table.rs +++ b/src/mutation_table.rs @@ -1,6 +1,7 @@ use crate::bindings as ll_bindings; use crate::metadata; -use crate::{tsk_id_t, tsk_size_t, TskitError}; +use crate::SizeType; +use crate::{tsk_id_t, TskitError}; use crate::{MutationId, NodeId, SiteId}; /// Row of a [`MutationTable`] @@ -27,7 +28,12 @@ impl PartialEq for MutationTableRow { } fn make_mutation_table_row(table: &MutationTable, pos: tsk_id_t) -> Option { - if pos < table.num_rows() as tsk_id_t { + use std::convert::TryFrom; + // panic is okay here, as we are handling a bad + // input value before we first call this to + // set up the iterator + let p = crate::SizeType::try_from(pos).unwrap(); + if p < table.num_rows() { let rv = MutationTableRow { id: pos.into(), site: table.site(pos).unwrap(), @@ -81,8 +87,8 @@ impl<'a> MutationTable<'a> { } /// Return the number of rows. - pub fn num_rows(&'a self) -> tsk_size_t { - self.table_.num_rows + pub fn num_rows(&'a self) -> SizeType { + self.table_.num_rows.into() } /// Return the ``site`` value from row ``row`` of the table. @@ -178,6 +184,10 @@ impl<'a> MutationTable<'a> { /// /// [`TskitError::IndexError`] if `r` is out of range. pub fn row + Copy>(&self, r: M) -> Result { - table_row_access!(r.into().0, self, make_mutation_table_row) + let ri = r.into(); + if ri < 0 { + return Err(crate::TskitError::IndexError); + } + table_row_access!(ri.0, self, make_mutation_table_row) } } diff --git a/src/node_table.rs b/src/node_table.rs index 170156ba0..0dd46b473 100644 --- a/src/node_table.rs +++ b/src/node_table.rs @@ -1,5 +1,6 @@ use crate::bindings as ll_bindings; use crate::metadata; +use crate::SizeType; use crate::{tsk_flags_t, tsk_id_t, TskitError}; use crate::{IndividualId, NodeId, PopulationId}; @@ -25,7 +26,12 @@ impl PartialEq for NodeTableRow { } fn make_node_table_row(table: &NodeTable, pos: tsk_id_t) -> Option { - if pos < table.num_rows() as tsk_id_t { + use std::convert::TryFrom; + // panic is okay here, as we are handling a bad + // input value before we first call this to + // set up the iterator + let p = crate::SizeType::try_from(pos).unwrap(); + if p < table.num_rows() { Some(NodeTableRow { id: pos.into(), time: table.time(pos).unwrap(), @@ -77,8 +83,8 @@ impl<'a> NodeTable<'a> { } /// Return the number of rows - pub fn num_rows(&'a self) -> ll_bindings::tsk_size_t { - self.table_.num_rows + pub fn num_rows(&'a self) -> SizeType { + self.table_.num_rows.into() } /// Return the ``time`` value from row ``row`` of the table. @@ -183,7 +189,11 @@ impl<'a> NodeTable<'a> { /// /// [`TskitError::IndexError`] if `r` is out of range. pub fn row + Copy>(&self, r: N) -> Result { - table_row_access!(r.into().0, self, make_node_table_row) + let ri = r.into(); + if ri < 0 { + return Err(crate::TskitError::IndexError); + } + table_row_access!(ri.0, self, make_node_table_row) } /// Obtain a vector containing the indexes ("ids") diff --git a/src/population_table.rs b/src/population_table.rs index ba7b28066..43908b78c 100644 --- a/src/population_table.rs +++ b/src/population_table.rs @@ -1,8 +1,9 @@ use crate::bindings as ll_bindings; use crate::metadata; +use crate::tsk_id_t; use crate::PopulationId; +use crate::SizeType; use crate::TskitError; -use crate::{tsk_id_t, tsk_size_t}; /// Row of a [`PopulationTable`] #[derive(Eq)] @@ -18,7 +19,12 @@ impl PartialEq for PopulationTableRow { } fn make_population_table_row(table: &PopulationTable, pos: tsk_id_t) -> Option { - if pos < table.num_rows() as tsk_id_t { + use std::convert::TryFrom; + // panic is okay here, as we are handling a bad + // input value before we first call this to + // set up the iterator + let p = crate::SizeType::try_from(pos).unwrap(); + if p < table.num_rows() { let rv = PopulationTableRow { id: pos.into(), metadata: table_row_decode_metadata!(table, pos), @@ -69,8 +75,8 @@ impl<'a> PopulationTable<'a> { } /// Return the number of rows. - pub fn num_rows(&'a self) -> tsk_size_t { - self.table_.num_rows + pub fn num_rows(&'a self) -> SizeType { + self.table_.num_rows.into() } pub fn metadata( @@ -100,6 +106,10 @@ impl<'a> PopulationTable<'a> { &self, r: P, ) -> Result { - table_row_access!(r.into().0, self, make_population_table_row) + let ri = r.into(); + if ri < 0 { + return Err(crate::TskitError::IndexError); + } + table_row_access!(ri.0, self, make_population_table_row) } } diff --git a/src/prelude.rs b/src/prelude.rs index 74dc08d7f..74d9543fe 100644 --- a/src/prelude.rs +++ b/src/prelude.rs @@ -1,8 +1,6 @@ //! Export commonly-use types and traits pub use crate::tsk_flags_t; -pub use crate::tsk_id_t; -pub use crate::tsk_size_t; pub use crate::NodeListGenerator; pub use crate::TableAccess; pub use crate::TskitTypeAccess; @@ -11,5 +9,5 @@ pub use streaming_iterator::DoubleEndedStreamingIterator; pub use streaming_iterator::StreamingIterator; pub use { crate::EdgeId, crate::IndividualId, crate::MigrationId, crate::MutationId, crate::NodeId, - crate::PopulationId, crate::SiteId, + crate::PopulationId, crate::SiteId, crate::SizeType, }; diff --git a/src/provenance.rs b/src/provenance.rs index 0d2ed8d32..bd8327e4b 100644 --- a/src/provenance.rs +++ b/src/provenance.rs @@ -11,6 +11,7 @@ //! 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. @@ -130,7 +131,12 @@ impl std::fmt::Display for ProvenanceTableRow { } fn make_provenance_table_row(table: &ProvenanceTable, pos: tsk_id_t) -> Option { - if pos < table.num_rows() as tsk_id_t { + use std::convert::TryFrom; + // panic is okay here, as we are handling a bad + // input value before we first call this to + // set up the iterator + let p = crate::SizeType::try_from(pos).unwrap(); + if p < table.num_rows() { Some(ProvenanceTableRow { id: pos.into(), timestamp: table.timestamp(pos).unwrap(), @@ -186,8 +192,8 @@ impl<'a> ProvenanceTable<'a> { } /// Return the number of rows - pub fn num_rows(&'a self) -> ll_bindings::tsk_size_t { - self.table_.num_rows + pub fn num_rows(&'a self) -> SizeType { + self.table_.num_rows.into() } /// Get the ISO-formatted time stamp for row `row`. @@ -291,7 +297,7 @@ mod test_provenance_tables { assert!(row_id == ProvenanceId(i as crate::tsk_id_t)); assert_eq!(tables.provenances().record(row_id).unwrap(), *r); } - assert_eq!(tables.provenances().num_rows() as usize, records.len()); + assert_eq!(usize::from(tables.provenances().num_rows()), records.len()); for (i, row) in tables.provenances_iter().enumerate() { assert_eq!(records[i], row.record); } diff --git a/src/site_table.rs b/src/site_table.rs index 8ba755447..82402db06 100644 --- a/src/site_table.rs +++ b/src/site_table.rs @@ -1,8 +1,9 @@ use crate::bindings as ll_bindings; use crate::metadata; +use crate::tsk_id_t; use crate::SiteId; +use crate::SizeType; use crate::TskitError; -use crate::{tsk_id_t, tsk_size_t}; /// Row of a [`SiteTable`] pub struct SiteTableRow { @@ -22,7 +23,12 @@ impl PartialEq for SiteTableRow { } fn make_site_table_row(table: &SiteTable, pos: tsk_id_t) -> Option { - if pos < table.num_rows() as tsk_id_t { + use std::convert::TryFrom; + // panic is okay here, as we are handling a bad + // input value before we first call this to + // set up the iterator + let p = crate::SizeType::try_from(pos).unwrap(); + if p < table.num_rows() { let rv = SiteTableRow { id: pos.into(), position: table.position(pos).unwrap(), @@ -73,8 +79,8 @@ impl<'a> SiteTable<'a> { } /// Return the number of rows - pub fn num_rows(&'a self) -> tsk_size_t { - self.table_.num_rows + pub fn num_rows(&'a self) -> SizeType { + self.table_.num_rows.into() } /// Return the ``position`` value from row ``row`` of the table. @@ -134,6 +140,10 @@ impl<'a> SiteTable<'a> { /// /// [`TskitError::IndexError`] if `r` is out of range. pub fn row + Copy>(&self, r: S) -> Result { + let ri = r.into(); + if ri < 0 { + return Err(crate::TskitError::IndexError); + } table_row_access!(r.into().0, self, make_site_table_row) } } diff --git a/src/table_collection.rs b/src/table_collection.rs index e9452a21f..750702716 100644 --- a/src/table_collection.rs +++ b/src/table_collection.rs @@ -211,7 +211,7 @@ impl TableCollection { parent.into().0, child.into().0, md.as_ptr(), - md.len(), + md.len().into(), ) }; @@ -258,7 +258,7 @@ impl TableCollection { parents.as_ptr() as *const tsk_id_t, parents.len() as tsk_size_t, md.as_ptr(), - md.len(), + md.len().into(), ) }; handle_tsk_return_value!(rv, IndividualId::from(rv)) @@ -323,7 +323,7 @@ impl TableCollection { source_dest.1.into().0, time, md.as_ptr(), - md.len(), + md.len().into(), ) }; handle_tsk_return_value!(rv, MigrationId(rv)) @@ -374,7 +374,7 @@ impl TableCollection { population.into().0, individual.into().0, md.as_ptr(), - md.len(), + md.len().into(), ) }; @@ -420,7 +420,7 @@ impl TableCollection { astate.0, astate.1, md.as_ptr(), - md.len(), + md.len().into(), ) }; @@ -481,7 +481,7 @@ impl TableCollection { dstate.0, dstate.1, md.as_ptr(), - md.len(), + md.len().into(), ) }; handle_tsk_return_value!(rv, MutationId::from(rv)) @@ -510,7 +510,7 @@ impl TableCollection { ll_bindings::tsk_population_table_add_row( &mut (*self.inner).populations, md.as_ptr(), - md.len(), + md.len().into(), ) }; @@ -665,7 +665,7 @@ impl TableCollection { ) -> Result>, TskitError> { let mut output_node_map: Vec = vec![]; if idmap { - output_node_map.resize(self.nodes().num_rows() as usize, NodeId::NULL); + output_node_map.resize(usize::from(self.nodes().num_rows()), NodeId::NULL); } let rv = unsafe { ll_bindings::tsk_table_collection_simplify( @@ -926,21 +926,21 @@ mod test { assert!(tables.is_indexed()); assert_eq!( tables.edge_insertion_order().unwrap().len(), - tables.edges().num_rows() as usize + tables.edges().num_rows().into() ); assert_eq!( tables.edge_removal_order().unwrap().len(), - tables.edges().num_rows() as usize + tables.edges().num_rows().into() ); for i in tables.edge_insertion_order().unwrap() { assert!(*i >= 0); - assert!(*i < tables.edges().num_rows() as tsk_id_t); + assert!(*i < tables.edges().num_rows()); } for i in tables.edge_removal_order().unwrap() { assert!(*i >= 0); - assert!(*i < tables.edges().num_rows() as tsk_id_t); + assert!(*i < tables.edges().num_rows()); } // The "transparent" casts are such black magic that we @@ -1048,6 +1048,8 @@ mod test { #[test] fn test_add_mutation() { + use std::convert::TryFrom; + let mut tables = TableCollection::new(1000.).unwrap(); tables @@ -1120,7 +1122,10 @@ mod test { for _ in tables.mutations().iter().skip(1) { nmuts += 1; } - assert_eq!(nmuts, tables.mutations().num_rows() - 1); + assert_eq!( + crate::SizeType::try_from(nmuts + 1).unwrap(), + tables.mutations().num_rows() + ); } struct F { @@ -1181,7 +1186,7 @@ mod test { let mut num_with_metadata = 0; let mut num_without_metadata = 0; - for i in 0..tables.mutations().num_rows() { + for i in 0..usize::from(tables.mutations().num_rows()) { match tables .mutations() .metadata::((i as tsk_id_t).into()) @@ -1656,15 +1661,18 @@ mod test_adding_site { Some(metadata[mi]) ); } - for i in 0..tables.sites().num_rows() as tsk_id_t { + for i in 0..usize::from(tables.sites().num_rows()) { assert!( - tables.sites().row(SiteId::from(i)).unwrap() - == tables.sites().row(SiteId::from(i)).unwrap() + tables.sites().row(SiteId::from(i as tsk_id_t)).unwrap() + == tables.sites().row(SiteId::from(i as tsk_id_t)).unwrap() ); if i > 0 { assert!( - tables.sites().row(SiteId::from(i)).unwrap() - != tables.sites().row(SiteId::from(i - 1)).unwrap() + tables.sites().row(SiteId::from(i as tsk_id_t)).unwrap() + != tables + .sites() + .row(SiteId::from((i - 1) as tsk_id_t)) + .unwrap() ); } } @@ -1749,6 +1757,7 @@ mod test_adding_migrations { #[test] fn test_add_migration_with_metadata() { use crate::metadata::MetadataRoundtrip; + use std::convert::TryInto; let metadata = vec![GenericMetadata::default(), GenericMetadata { data: 84 }]; @@ -1775,7 +1784,7 @@ mod test_adding_migrations { assert_eq!(row.node, NodeId(7 * id_i)); } - for i in 0..tables.migrations().num_rows() as tsk_id_t { + for i in 0..tables.migrations().num_rows().try_into().unwrap() { assert!( tables.migrations().row(MigrationId::from(i)).unwrap() == tables.migrations().row(MigrationId::from(i)).unwrap() diff --git a/src/traits.rs b/src/traits.rs index 92cb94833..035e17d79 100644 --- a/src/traits.rs +++ b/src/traits.rs @@ -145,7 +145,7 @@ pub trait NodeListGenerator: TableAccess { /// Get all nodes with time > 0.0: /// /// ``` - /// use tskit::tsk_id_t; + /// use tskit::bindings::tsk_id_t; /// use tskit::TableAccess; /// use tskit::NodeListGenerator; /// diff --git a/src/trees.rs b/src/trees.rs index 335f20f4c..afa8a0f78 100644 --- a/src/trees.rs +++ b/src/trees.rs @@ -10,6 +10,7 @@ use crate::NodeTable; use crate::PopulationTable; use crate::SimplificationOptions; use crate::SiteTable; +use crate::SizeType; use crate::TableAccess; use crate::TableOutputOptions; use crate::TreeFlags; @@ -68,7 +69,7 @@ impl Tree { rv = unsafe { ll_bindings::tsk_tree_set_tracked_samples( tree.inner, - ts.num_samples() as u64, + ts.num_samples().into(), (*tree.inner).samples, ) }; @@ -1095,8 +1096,8 @@ impl TreeSequence { } /// Get the number of trees. - pub fn num_trees(&self) -> tsk_size_t { - unsafe { ll_bindings::tsk_treeseq_get_num_trees(self.inner) } + pub fn num_trees(&self) -> SizeType { + unsafe { ll_bindings::tsk_treeseq_get_num_trees(self.inner) }.into() } /// Calculate the average Kendall-Colijn (`K-C`) distance between @@ -1119,8 +1120,8 @@ impl TreeSequence { } // FIXME: document - pub fn num_samples(&self) -> tsk_size_t { - unsafe { ll_bindings::tsk_treeseq_get_num_samples(self.inner) } + pub fn num_samples(&self) -> SizeType { + unsafe { ll_bindings::tsk_treeseq_get_num_samples(self.inner) }.into() } /// Simplify tables and return a new tree sequence. @@ -1147,7 +1148,7 @@ impl TreeSequence { let ts = tables.tree_sequence(TreeSequenceFlags::default())?; let mut output_node_map: Vec = vec![]; if idmap { - output_node_map.resize(self.nodes().num_rows() as usize, NodeId::NULL); + output_node_map.resize(usize::from(self.nodes().num_rows()), NodeId::NULL); } let rv = unsafe { ll_bindings::tsk_treeseq_simplify(