Skip to content

Commit f8a7814

Browse files
molpopgenmolpopgen
authored
refactor: remove lifetime annotation from table types (#373)
* Types like EdgeTable no longer require lifetimes. * tskit::table_views::TableViews is now a Deref target of TableCollection and TreeSequence. * This Deref target replaces the behaviors of the TableAccess and NodeListGenerator traits, which have been removed. * TableViews is a DerefMut target of TableCollection. * The removed traits affect the contents of the crate prelude. * Some code that would not compile due to dropped temporaries will now compile. (Doc tests updated accordingly.) * The new fn nodes_mut addresses API breakage. These changes are a BIG WIN for overall design. BREAKING CHANGE: traits replaced with Deref targets
1 parent d5ade92 commit f8a7814

21 files changed

+744
-571
lines changed

‎examples/forward_simulation.rs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ use rand::rngs::StdRng;
2323
use rand::Rng;
2424
use rand::SeedableRng;
2525
use rand_distr::{Exp, Uniform};
26-
use tskit::TableAccess;
2726
use tskit::TskitTypeAccess;
2827

2928
#[derive(clap::Parser)]

‎src/_macros.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -518,7 +518,7 @@ macro_rules! build_owned_tables {
518518
}
519519

520520
impl std::ops::Deref for $name {
521-
type Target = $deref<'static>;
521+
type Target = $deref;
522522

523523
fn deref(&self) -> &Self::Target {
524524
// SAFETY: that T* and &T have same layout,

‎src/edge_table.rs‎

Lines changed: 50 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
use std::ptr::NonNull;
2+
13
use crate::bindings as ll_bindings;
24
use crate::metadata;
35
use crate::Position;
@@ -28,7 +30,7 @@ impl PartialEq for EdgeTableRow {
2830
}
2931

3032
fn make_edge_table_row(table: &EdgeTable, pos: tsk_id_t) -> Option<EdgeTableRow> {
31-
let table_ref = table.table_;
33+
let table_ref = table.as_ref();
3234
Some(EdgeTableRow {
3335
id: pos.into(),
3436
left: table.left(pos)?,
@@ -39,8 +41,8 @@ fn make_edge_table_row(table: &EdgeTable, pos: tsk_id_t) -> Option<EdgeTableRow>
3941
})
4042
}
4143

42-
pub(crate) type EdgeTableRefIterator<'a> = crate::table_iterator::TableIterator<&'a EdgeTable<'a>>;
43-
pub(crate) type EdgeTableIterator<'a> = crate::table_iterator::TableIterator<EdgeTable<'a>>;
44+
pub(crate) type EdgeTableRefIterator<'a> = crate::table_iterator::TableIterator<&'a EdgeTable>;
45+
pub(crate) type EdgeTableIterator = crate::table_iterator::TableIterator<EdgeTable>;
4446

4547
impl<'a> Iterator for EdgeTableRefIterator<'a> {
4648
type Item = EdgeTableRow;
@@ -52,7 +54,7 @@ impl<'a> Iterator for EdgeTableRefIterator<'a> {
5254
}
5355
}
5456

55-
impl<'a> Iterator for EdgeTableIterator<'a> {
57+
impl Iterator for EdgeTableIterator {
5658
type Item = EdgeTableRow;
5759

5860
fn next(&mut self) -> Option<Self::Item> {
@@ -64,22 +66,32 @@ impl<'a> Iterator for EdgeTableIterator<'a> {
6466

6567
/// An immutable view of an edge table.
6668
///
67-
/// These are not created directly.
68-
/// Instead, use [`TableAccess::edges`](crate::TableAccess::edges)
69-
/// to get a reference to an existing edge table;
69+
/// These are not created directly but are accessed
70+
/// by types implementing [`std::ops::Deref`] to
71+
/// [`crate::table_views::TableViews`]
7072
#[repr(transparent)]
71-
pub struct EdgeTable<'a> {
72-
table_: &'a ll_bindings::tsk_edge_table_t,
73+
pub struct EdgeTable {
74+
pub(crate) table_: NonNull<ll_bindings::tsk_edge_table_t>,
7375
}
7476

75-
impl<'a> EdgeTable<'a> {
76-
pub(crate) fn new_from_table(edges: &'a ll_bindings::tsk_edge_table_t) -> Self {
77-
EdgeTable { table_: edges }
77+
impl EdgeTable {
78+
pub(crate) fn new_from_table(
79+
edges: *mut ll_bindings::tsk_edge_table_t,
80+
) -> Result<Self, TskitError> {
81+
let n = NonNull::new(edges).ok_or_else(|| {
82+
TskitError::LibraryError("null pointer to tsk_edge_table_t".to_string())
83+
})?;
84+
Ok(EdgeTable { table_: n })
85+
}
86+
87+
pub(crate) fn as_ref(&self) -> &ll_bindings::tsk_edge_table_t {
88+
// SAFETY: NonNull
89+
unsafe { self.table_.as_ref() }
7890
}
7991

8092
/// Return the number of rows
81-
pub fn num_rows(&'a self) -> crate::SizeType {
82-
self.table_.num_rows.into()
93+
pub fn num_rows(&self) -> crate::SizeType {
94+
self.as_ref().num_rows.into()
8395
}
8496

8597
/// Return the ``parent`` value from row ``row`` of the table.
@@ -88,12 +100,12 @@ impl<'a> EdgeTable<'a> {
88100
///
89101
/// * `Some(parent)` if `u` is valid.
90102
/// * `None` otherwise.
91-
pub fn parent<E: Into<EdgeId> + Copy>(&'a self, row: E) -> Option<NodeId> {
103+
pub fn parent<E: Into<EdgeId> + Copy>(&self, row: E) -> Option<NodeId> {
92104
unsafe_tsk_column_access!(
93105
row.into().0,
94106
0,
95107
self.num_rows(),
96-
self.table_,
108+
self.as_ref(),
97109
parent,
98110
NodeId
99111
)
@@ -105,8 +117,15 @@ impl<'a> EdgeTable<'a> {
105117
///
106118
/// * `Some(child)` if `u` is valid.
107119
/// * `None` otherwise.
108-
pub fn child<E: Into<EdgeId> + Copy>(&'a self, row: E) -> Option<NodeId> {
109-
unsafe_tsk_column_access!(row.into().0, 0, self.num_rows(), self.table_, child, NodeId)
120+
pub fn child<E: Into<EdgeId> + Copy>(&self, row: E) -> Option<NodeId> {
121+
unsafe_tsk_column_access!(
122+
row.into().0,
123+
0,
124+
self.num_rows(),
125+
self.as_ref(),
126+
child,
127+
NodeId
128+
)
110129
}
111130

112131
/// Return the ``left`` value from row ``row`` of the table.
@@ -115,12 +134,12 @@ impl<'a> EdgeTable<'a> {
115134
///
116135
/// * `Some(position)` if `u` is valid.
117136
/// * `None` otherwise.
118-
pub fn left<E: Into<EdgeId> + Copy>(&'a self, row: E) -> Option<Position> {
137+
pub fn left<E: Into<EdgeId> + Copy>(&self, row: E) -> Option<Position> {
119138
unsafe_tsk_column_access!(
120139
row.into().0,
121140
0,
122141
self.num_rows(),
123-
self.table_,
142+
self.as_ref(),
124143
left,
125144
Position
126145
)
@@ -132,8 +151,14 @@ impl<'a> EdgeTable<'a> {
132151
///
133152
/// * `Some(position)` if `u` is valid.
134153
/// * `None` otherwise.
135-
pub fn right<E: Into<EdgeId> + Copy>(&'a self, row: E) -> Option<Position> {
136-
unsafe_tsk_column_access_and_map_into!(row.into().0, 0, self.num_rows(), self.table_, right)
154+
pub fn right<E: Into<EdgeId> + Copy>(&self, row: E) -> Option<Position> {
155+
unsafe_tsk_column_access_and_map_into!(
156+
row.into().0,
157+
0,
158+
self.num_rows(),
159+
self.as_ref(),
160+
right
161+
)
137162
}
138163

139164
/// Retrieve decoded metadata for a `row`.
@@ -153,10 +178,10 @@ impl<'a> EdgeTable<'a> {
153178
/// The big-picture semantics are the same for all table types.
154179
/// See [`crate::IndividualTable::metadata`] for examples.
155180
pub fn metadata<T: metadata::EdgeMetadata>(
156-
&'a self,
181+
&self,
157182
row: EdgeId,
158183
) -> Option<Result<T, TskitError>> {
159-
let table_ref = self.table_;
184+
let table_ref = self.as_ref();
160185
let buffer = metadata_to_vector!(self, table_ref, row.0)?;
161186
Some(decode_metadata_row!(T, buffer).map_err(|e| e.into()))
162187
}
@@ -165,7 +190,7 @@ impl<'a> EdgeTable<'a> {
165190
/// The value of the iterator is [`EdgeTableRow`].
166191
///
167192
pub fn iter(&self) -> impl Iterator<Item = EdgeTableRow> + '_ {
168-
crate::table_iterator::make_table_iterator::<&EdgeTable<'a>>(self)
193+
crate::table_iterator::make_table_iterator::<&EdgeTable>(self)
169194
}
170195

171196
/// Return row `r` of the table.

‎src/individual_table.rs‎

Lines changed: 42 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
use std::ptr::NonNull;
2+
13
use crate::bindings as ll_bindings;
24
use crate::metadata;
35
use crate::IndividualFlags;
@@ -45,15 +47,15 @@ impl PartialEq for IndividualTableRow {
4547

4648
/// An immutable view of a individual table.
4749
///
48-
/// These are not created directly.
49-
/// Instead, use [`TableAccess::individuals`](crate::TableAccess::individuals)
50-
/// to get a reference to an existing node table;
51-
pub struct IndividualTable<'a> {
52-
table_: &'a ll_bindings::tsk_individual_table_t,
50+
/// These are not created directly but are accessed
51+
/// by types implementing [`std::ops::Deref`] to
52+
/// [`crate::table_views::TableViews`]
53+
pub struct IndividualTable {
54+
table_: NonNull<ll_bindings::tsk_individual_table_t>,
5355
}
5456

5557
fn make_individual_table_row(table: &IndividualTable, pos: tsk_id_t) -> Option<IndividualTableRow> {
56-
let table_ref = table.table_;
58+
let table_ref = table.as_ref();
5759
Some(IndividualTableRow {
5860
id: pos.into(),
5961
flags: table.flags(pos)?,
@@ -64,9 +66,8 @@ fn make_individual_table_row(table: &IndividualTable, pos: tsk_id_t) -> Option<I
6466
}
6567

6668
pub(crate) type IndividualTableRefIterator<'a> =
67-
crate::table_iterator::TableIterator<&'a IndividualTable<'a>>;
68-
pub(crate) type IndividualTableIterator<'a> =
69-
crate::table_iterator::TableIterator<IndividualTable<'a>>;
69+
crate::table_iterator::TableIterator<&'a IndividualTable>;
70+
pub(crate) type IndividualTableIterator = crate::table_iterator::TableIterator<IndividualTable>;
7071

7172
impl<'a> Iterator for IndividualTableRefIterator<'a> {
7273
type Item = IndividualTableRow;
@@ -78,7 +79,7 @@ impl<'a> Iterator for IndividualTableRefIterator<'a> {
7879
}
7980
}
8081

81-
impl<'a> Iterator for IndividualTableIterator<'a> {
82+
impl Iterator for IndividualTableIterator {
8283
type Item = IndividualTableRow;
8384

8485
fn next(&mut self) -> Option<Self::Item> {
@@ -88,16 +89,24 @@ impl<'a> Iterator for IndividualTableIterator<'a> {
8889
}
8990
}
9091

91-
impl<'a> IndividualTable<'a> {
92-
pub(crate) fn new_from_table(individuals: &'a ll_bindings::tsk_individual_table_t) -> Self {
93-
IndividualTable {
94-
table_: individuals,
95-
}
92+
impl IndividualTable {
93+
pub(crate) fn new_from_table(
94+
individuals: *mut ll_bindings::tsk_individual_table_t,
95+
) -> Result<Self, TskitError> {
96+
let n = NonNull::new(individuals).ok_or_else(|| {
97+
TskitError::LibraryError("null pointer to tsk_individual_table_t".to_string())
98+
})?;
99+
Ok(IndividualTable { table_: n })
100+
}
101+
102+
pub(crate) fn as_ref(&self) -> &ll_bindings::tsk_individual_table_t {
103+
// SAFETY: NonNull
104+
unsafe { self.table_.as_ref() }
96105
}
97106

98107
/// Return the number of rows
99-
pub fn num_rows(&'a self) -> crate::SizeType {
100-
self.table_.num_rows.into()
108+
pub fn num_rows(&self) -> crate::SizeType {
109+
self.as_ref().num_rows.into()
101110
}
102111

103112
/// Return the flags for a given row.
@@ -107,7 +116,13 @@ impl<'a> IndividualTable<'a> {
107116
/// * `Some(flags)` if `row` is valid.
108117
/// * `None` otherwise.
109118
pub fn flags<I: Into<IndividualId> + Copy>(&self, row: I) -> Option<IndividualFlags> {
110-
unsafe_tsk_column_access_and_map_into!(row.into().0, 0, self.num_rows(), self.table_, flags)
119+
unsafe_tsk_column_access_and_map_into!(
120+
row.into().0,
121+
0,
122+
self.num_rows(),
123+
self.as_ref(),
124+
flags
125+
)
111126
}
112127

113128
/// Return the locations for a given row.
@@ -121,7 +136,7 @@ impl<'a> IndividualTable<'a> {
121136
row.into().0,
122137
0,
123138
self.num_rows(),
124-
self.table_,
139+
self.as_ref(),
125140
location,
126141
location_offset,
127142
location_length,
@@ -140,7 +155,7 @@ impl<'a> IndividualTable<'a> {
140155
row.into().0,
141156
0,
142157
self.num_rows(),
143-
self.table_,
158+
self.as_ref(),
144159
parents,
145160
parents_offset,
146161
parents_length,
@@ -179,7 +194,7 @@ impl<'a> IndividualTable<'a> {
179194
///
180195
/// ```
181196
/// # #[cfg(feature = "derive")] {
182-
/// # use tskit::TableAccess;
197+
/// #
183198
/// # let mut tables = tskit::TableCollection::new(100.).unwrap();
184199
/// # #[derive(serde::Serialize, serde::Deserialize, tskit::metadata::IndividualMetadata)]
185200
/// # #[serializer("serde_json")]
@@ -202,7 +217,7 @@ impl<'a> IndividualTable<'a> {
202217
///
203218
/// ```
204219
/// # #[cfg(feature = "derive")] {
205-
/// # use tskit::TableAccess;
220+
/// #
206221
/// # let mut tables = tskit::TableCollection::new(100.).unwrap();
207222
/// # #[derive(serde::Serialize, serde::Deserialize, tskit::metadata::IndividualMetadata)]
208223
/// # #[serializer("serde_json")]
@@ -248,7 +263,7 @@ impl<'a> IndividualTable<'a> {
248263
# struct MutationMetadata {
249264
# x: i32,
250265
# }
251-
# use tskit::TableAccess;
266+
#
252267
# let mut tables = tskit::TableCollection::new(10.).unwrap();
253268
match tables.individuals().metadata::<MutationMetadata>(0.into())
254269
{
@@ -280,7 +295,7 @@ match tables.individuals().metadata::<MutationMetadata>(0.into())
280295
///
281296
/// ```
282297
/// # #[cfg(feature = "derive")] {
283-
/// # use tskit::TableAccess;
298+
/// #
284299
/// # #[derive(serde::Serialize, serde::Deserialize, tskit::metadata::IndividualMetadata)]
285300
/// # #[serializer("serde_json")]
286301
/// # struct IndividualMetadata {
@@ -319,10 +334,10 @@ match tables.individuals().metadata::<MutationMetadata>(0.into())
319334
/// 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
320335
/// metadata type for a table.
321336
pub fn metadata<T: metadata::IndividualMetadata>(
322-
&'a self,
337+
&self,
323338
row: IndividualId,
324339
) -> Option<Result<T, TskitError>> {
325-
let table_ref = self.table_;
340+
let table_ref = self.as_ref();
326341
let buffer = metadata_to_vector!(self, table_ref, row.0)?;
327342
Some(decode_metadata_row!(T, buffer).map_err(|e| e.into()))
328343
}
@@ -331,7 +346,7 @@ match tables.individuals().metadata::<MutationMetadata>(0.into())
331346
/// The value of the iterator is [`IndividualTableRow`].
332347
///
333348
pub fn iter(&self) -> impl Iterator<Item = IndividualTableRow> + '_ {
334-
crate::table_iterator::make_table_iterator::<&IndividualTable<'a>>(self)
349+
crate::table_iterator::make_table_iterator::<&IndividualTable>(self)
335350
}
336351

337352
/// Return row `r` of the table.

‎src/lib.rs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ pub mod prelude;
9292
mod site_table;
9393
mod table_collection;
9494
mod table_iterator;
95+
pub mod table_views;
9596
mod traits;
9697
mod tree_interface;
9798
mod trees;
@@ -438,8 +439,6 @@ pub use site_table::{OwnedSiteTable, SiteTable, SiteTableRow};
438439
pub use table_collection::TableCollection;
439440
pub use traits::IndividualLocation;
440441
pub use traits::IndividualParents;
441-
pub use traits::NodeListGenerator;
442-
pub use traits::TableAccess;
443442
pub use traits::TskitTypeAccess;
444443
pub use tree_interface::{NodeTraversalOrder, TreeInterface};
445444
pub use trees::{Tree, TreeSequence};

‎src/metadata.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
//! ```
3131
//! # #[cfg(feature = "derive")] {
3232
//! use tskit::handle_metadata_return;
33-
//! use tskit::TableAccess;
33+
//!
3434
//!
3535
//! #[derive(serde::Serialize, serde::Deserialize, tskit::metadata::MutationMetadata)]
3636
//! #[serializer("serde_json")]
@@ -75,7 +75,7 @@
7575
//!
7676
//! ```
7777
//! # #[cfg(feature = "derive")] {
78-
//! use tskit::TableAccess;
78+
//!
7979
//! #[derive(serde::Serialize, serde::Deserialize, PartialEq, PartialOrd)]
8080
//! struct GeneticValue(f64);
8181
//!
@@ -101,7 +101,7 @@
101101
//! For fun, we'll use the Python [`pickle`](https://docs.rs/crate/serde-pickle/) format.
102102
//!
103103
//! ```
104-
//! use tskit::TableAccess;
104+
//!
105105
//!
106106
//! #[derive(serde::Serialize, serde::Deserialize)]
107107
//! struct Metadata {

0 commit comments

Comments
 (0)