From ee004bac19a992e9810a24af3bcf64d922b19993 Mon Sep 17 00:00:00 2001 From: Michael Bradley Date: Fri, 10 Jan 2025 20:02:52 +1300 Subject: [PATCH 1/4] Rename Item -> containers::Pair --- src/backing/containers/mod.rs | 3 +++ src/backing/{item.rs => containers/pair.rs} | 12 ++++++------ src/backing/indexed/mod.rs | 4 ++-- src/backing/mod.rs | 2 +- src/queue/paired.rs | 16 ++++++++-------- 5 files changed, 20 insertions(+), 17 deletions(-) create mode 100644 src/backing/containers/mod.rs rename src/backing/{item.rs => containers/pair.rs} (75%) diff --git a/src/backing/containers/mod.rs b/src/backing/containers/mod.rs new file mode 100644 index 0000000..79f8058 --- /dev/null +++ b/src/backing/containers/mod.rs @@ -0,0 +1,3 @@ +mod pair; + +pub use pair::Pair; diff --git a/src/backing/item.rs b/src/backing/containers/pair.rs similarity index 75% rename from src/backing/item.rs rename to src/backing/containers/pair.rs index 8c926a5..a204cf9 100644 --- a/src/backing/item.rs +++ b/src/backing/containers/pair.rs @@ -3,12 +3,12 @@ use std::cmp::Ordering; /// Helper struct to associate an item with its priority #[derive(Debug, Clone, Copy)] // I mean I guess P should be Ord but I want to use f64 so whatever -pub struct Item { +pub struct Pair { data: D, priority: P, } -impl Item { +impl Pair { /// Creates a new instance pub fn new(data: D, priority: P) -> Self { Self { data, priority } @@ -21,7 +21,7 @@ impl Item { } // The relevant Ord implementations are based just on the priority -impl Ord for Item { +impl Ord for Pair { fn cmp(&self, other: &Self) -> Ordering { // Yeah this is bad design // My excuse is that i'm still learning Rust @@ -31,16 +31,16 @@ impl Ord for Item { } } -impl PartialOrd for Item { +impl PartialOrd for Pair { fn partial_cmp(&self, other: &Self) -> Option { self.priority.partial_cmp(&other.priority) } } -impl PartialEq for Item { +impl PartialEq for Pair { fn eq(&self, other: &Self) -> bool { self.priority == other.priority } } -impl Eq for Item {} +impl Eq for Pair {} diff --git a/src/backing/indexed/mod.rs b/src/backing/indexed/mod.rs index ebc1ad5..c729567 100644 --- a/src/backing/indexed/mod.rs +++ b/src/backing/indexed/mod.rs @@ -1,9 +1,9 @@ /// Data structures for the "indexed" min-queues, supporting priority updates and arbitrary removals, but no duplicates -use super::{item::Item, pure::PureBacking}; +use super::{containers::Pair, pure::PureBacking}; /// A data structure usable for backing an "indexed" queue pub trait IndexedBacking: - PureBacking> + PureBacking> { /// Update an item's priority fn update(data: D, priority: P) -> Result<(), ()>; diff --git a/src/backing/mod.rs b/src/backing/mod.rs index 1c34cd3..65d0cb1 100644 --- a/src/backing/mod.rs +++ b/src/backing/mod.rs @@ -1,3 +1,3 @@ +pub mod containers; pub mod indexed; -pub mod item; pub mod pure; diff --git a/src/queue/paired.rs b/src/queue/paired.rs index 84f743f..611524e 100644 --- a/src/queue/paired.rs +++ b/src/queue/paired.rs @@ -1,6 +1,6 @@ /// A "paired" priority queue that links some data to a priority and supports duplicates, but not arbitrary deletions or weight updates use crate::backing::{ - item::Item, + containers::Pair, pure::{BinaryHeap, PureBacking}, }; use pyo3::{ @@ -11,7 +11,7 @@ use pyo3::{ #[pyclass] pub struct PairedQueue { - backing: Box, f64>>>, + backing: Box, f64>>>, } #[pymethods] @@ -62,12 +62,12 @@ impl PairedQueue { } fn __setitem__(mut self_: PyRefMut<'_, Self>, key: Py, value: f64) { - self_.backing.add(Item::new(key, value)); + self_.backing.add(Pair::new(key, value)); } } impl<'py> PairedQueue { - fn from_any(object: &Bound<'py, PyAny>) -> PyResult, f64>>> { + fn from_any(object: &Bound<'py, PyAny>) -> PyResult, f64>>> { if let Ok(vec) = object.extract::, f64)>>() { Ok(Self::from_vec(vec)) } else { @@ -87,17 +87,17 @@ impl<'py> PairedQueue { } } - fn from_vec(list: Vec<(Py, f64)>) -> Vec, f64>> { + fn from_vec(list: Vec<(Py, f64)>) -> Vec, f64>> { list.into_iter() - .map(|(data, priority)| Item::new(data, priority)) + .map(|(data, priority)| Pair::new(data, priority)) .collect() } - fn from_dict(dict: &Bound<'py, PyDict>) -> PyResult, f64>>> { + fn from_dict(dict: &Bound<'py, PyDict>) -> PyResult, f64>>> { if let Ok(items) = dict .into_iter() .map(|(data, priority)| match priority.extract::() { - Ok(value) => Ok(Item::new(data.unbind(), value)), + Ok(value) => Ok(Pair::new(data.unbind(), value)), Err(err) => Err(err), }) .collect::, _>>() From 38a544db76f0cd0d7f52c69a4de6e89658804e7a Mon Sep 17 00:00:00 2001 From: Michael Bradley Date: Fri, 10 Jan 2025 20:53:28 +1300 Subject: [PATCH 2/4] Add PyItem container Also reduce queue item trait bound from Ord to just PartialOrd --- src/backing/containers/mod.rs | 2 ++ src/backing/containers/pair.rs | 19 ++----------- src/backing/containers/py_item.rs | 46 +++++++++++++++++++++++++++++++ src/backing/indexed/mod.rs | 2 +- src/backing/pure/binary_heap.rs | 18 ++++++------ src/backing/pure/mod.rs | 2 +- src/queue/paired.rs | 6 +++- src/queue/pure.rs | 28 ++++++++++++++++--- 8 files changed, 91 insertions(+), 32 deletions(-) create mode 100644 src/backing/containers/py_item.rs diff --git a/src/backing/containers/mod.rs b/src/backing/containers/mod.rs index 79f8058..74cfd32 100644 --- a/src/backing/containers/mod.rs +++ b/src/backing/containers/mod.rs @@ -1,3 +1,5 @@ mod pair; +mod py_item; pub use pair::Pair; +pub use py_item::PyItem; diff --git a/src/backing/containers/pair.rs b/src/backing/containers/pair.rs index a204cf9..d194835 100644 --- a/src/backing/containers/pair.rs +++ b/src/backing/containers/pair.rs @@ -1,8 +1,7 @@ use std::cmp::Ordering; -/// Helper struct to associate an item with its priority +/// Container to associate an item with a priority #[derive(Debug, Clone, Copy)] -// I mean I guess P should be Ord but I want to use f64 so whatever pub struct Pair { data: D, priority: P, @@ -14,23 +13,13 @@ impl Pair { Self { data, priority } } - /// Retrieve the internal data, it would be nicer to implement this using [`From`] or [`Into`], but I don't see a way to do that using generics + /// Retrieves the internal data. + /// It would be nicer to implement this using [`From`] or [`Into`], but I don't see a way to do that using generics. pub fn data(self) -> D { self.data } } -// The relevant Ord implementations are based just on the priority -impl Ord for Pair { - fn cmp(&self, other: &Self) -> Ordering { - // Yeah this is bad design - // My excuse is that i'm still learning Rust - self.priority - .partial_cmp(&other.priority) - .unwrap_or(Ordering::Equal) - } -} - impl PartialOrd for Pair { fn partial_cmp(&self, other: &Self) -> Option { self.priority.partial_cmp(&other.priority) @@ -42,5 +31,3 @@ impl PartialEq for Pair { self.priority == other.priority } } - -impl Eq for Pair {} diff --git a/src/backing/containers/py_item.rs b/src/backing/containers/py_item.rs new file mode 100644 index 0000000..9c7110c --- /dev/null +++ b/src/backing/containers/py_item.rs @@ -0,0 +1,46 @@ +use std::cmp::Ordering; + +use pyo3::prelude::*; + +/// Container that provides PartialOrd based on the Python object it holds +#[derive(Debug, Clone)] +pub struct PyItem(Py); + +impl PyItem { + /// Creates a new instance + fn new(item: Py) -> Self { + PyItem(item) + } + + /// Retrieves the internal data + fn item(self) -> Py { + self.0 + } +} + +impl PartialOrd for PyItem { + fn partial_cmp(&self, other: &Self) -> Option { + Python::with_gil(|py| { + // Bind objects for convenience + let ours = self.0.bind(py); + let theirs = other.0.bind(py); + + // Compare based on Python comparison implementations + match ours.lt(theirs) { + Ok(true) => Some(Ordering::Less), + Ok(false) => match ours.gt(theirs) { + Ok(true) => Some(Ordering::Greater), + Ok(false) => Some(Ordering::Equal), // If the python class is implemented strangely then this may be wrong + Err(_) => None, + }, + Err(_) => None, + } + }) + } +} + +impl PartialEq for PyItem { + fn eq(&self, other: &Self) -> bool { + Python::with_gil(|py| self.0.bind(py).eq(other.0.bind(py)).unwrap_or(false)) + } +} diff --git a/src/backing/indexed/mod.rs b/src/backing/indexed/mod.rs index c729567..6579634 100644 --- a/src/backing/indexed/mod.rs +++ b/src/backing/indexed/mod.rs @@ -2,7 +2,7 @@ use super::{containers::Pair, pure::PureBacking}; /// A data structure usable for backing an "indexed" queue -pub trait IndexedBacking: +pub trait IndexedBacking: PureBacking> { /// Update an item's priority diff --git a/src/backing/pure/binary_heap.rs b/src/backing/pure/binary_heap.rs index 6569a66..cd84f18 100644 --- a/src/backing/pure/binary_heap.rs +++ b/src/backing/pure/binary_heap.rs @@ -2,12 +2,6 @@ use std::fmt; use super::PureBacking; -/// A binary min-heap backed by an array -#[derive(Debug)] -pub struct BinaryHeap { - data: Vec, -} - /// Indicates why a sift failed #[derive(Debug, Clone)] struct SiftError { @@ -33,7 +27,13 @@ impl fmt::Display for SiftError { /// Whether a sift operation succeeded type SiftResult = Result<(), SiftError>; -impl BinaryHeap { +/// A binary min-heap backed by an array +#[derive(Debug)] +pub struct BinaryHeap { + data: Vec, +} + +impl BinaryHeap { /// Instantiates a new (empty) binary heap pub fn new() -> Self { Self { data: vec![] } @@ -108,7 +108,7 @@ impl BinaryHeap { } } -impl FromIterator for BinaryHeap { +impl FromIterator for BinaryHeap { fn from_iter>(iter: U) -> Self { let mut this = Self { data: Vec::from_iter(iter), @@ -120,7 +120,7 @@ impl FromIterator for BinaryHeap { } } -impl PureBacking for BinaryHeap { +impl PureBacking for BinaryHeap { fn add(&mut self, item: T) { // Append item self.data.push(item); diff --git a/src/backing/pure/mod.rs b/src/backing/pure/mod.rs index a8b0259..1ff9d9a 100644 --- a/src/backing/pure/mod.rs +++ b/src/backing/pure/mod.rs @@ -3,7 +3,7 @@ mod binary_heap; pub use binary_heap::BinaryHeap; /// A data structure usable for backing a "pure" queue -pub trait PureBacking: Send + Sync { +pub trait PureBacking: Send + Sync { /// Places an item into the queue fn add(&mut self, item: T); /// Removes the item with minimum priority, if it exists diff --git a/src/queue/paired.rs b/src/queue/paired.rs index 611524e..69aaee4 100644 --- a/src/queue/paired.rs +++ b/src/queue/paired.rs @@ -16,6 +16,7 @@ pub struct PairedQueue { #[pymethods] impl PairedQueue { + /// Enables generic typing #[classmethod] fn __class_getitem__(cls_: Bound<'_, PyType>, _key: Py) -> Bound<'_, PyType> { cls_ @@ -67,6 +68,7 @@ impl PairedQueue { } impl<'py> PairedQueue { + /// Tries to a Python object into a vector suitable for ingestion into the backing fn from_any(object: &Bound<'py, PyAny>) -> PyResult, f64>>> { if let Ok(vec) = object.extract::, f64)>>() { Ok(Self::from_vec(vec)) @@ -87,12 +89,14 @@ impl<'py> PairedQueue { } } + /// Converts a vector of Python objects and priorities into a vector of items fn from_vec(list: Vec<(Py, f64)>) -> Vec, f64>> { list.into_iter() .map(|(data, priority)| Pair::new(data, priority)) .collect() } + /// Converts a Python dictionary into a vector of items fn from_dict(dict: &Bound<'py, PyDict>) -> PyResult, f64>>> { if let Ok(items) = dict .into_iter() @@ -104,7 +108,7 @@ impl<'py> PairedQueue { { Ok(items) } else { - Err(PyErr::new::("Dict keys were not floats")) + Err(PyErr::new::("Dict keys were not numbers")) } } } diff --git a/src/queue/pure.rs b/src/queue/pure.rs index 8cf4912..d040ff8 100644 --- a/src/queue/pure.rs +++ b/src/queue/pure.rs @@ -1,12 +1,32 @@ -use pyo3::prelude::*; +use pyo3::{prelude::*, types::PyType}; + +use crate::backing::{ + containers::PyItem, + pure::{BinaryHeap, PureBacking}, +}; #[pyclass] -pub struct PureQueue {} +pub struct PureQueue { + backing: Box>, +} #[pymethods] impl PureQueue { + /// Enables generic typing + #[classmethod] + fn __class_getitem__(cls_: Bound<'_, PyType>, _key: Py) -> Bound<'_, PyType> { + cls_ + } + #[new] - fn new() -> Self { - Self {} + #[pyo3(signature = (items=None))] + fn new(items: Option>) -> PyResult { + if let Some(py_object) = items { + todo!() + } else { + Ok(Self { + backing: Box::new(BinaryHeap::new()), + }) + } } } From 608ff1a3d59469bcd30f1c041a99b205cf5d7051 Mon Sep 17 00:00:00 2001 From: Michael Bradley Date: Fri, 10 Jan 2025 21:21:12 +1300 Subject: [PATCH 3/4] Fix incorrect tests --- tests/indexed_queue.py | 2 +- tests/pure_queue.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/indexed_queue.py b/tests/indexed_queue.py index c829962..37b7cc2 100644 --- a/tests/indexed_queue.py +++ b/tests/indexed_queue.py @@ -27,7 +27,7 @@ def test_empty_creation(): {"a": 0, "b": 1, "c": 2}, )) def test_creation(items: IndexedQueueInitializer): - queue = IndexedQueue() + queue = IndexedQueue(items) assert len(queue) == len(items) diff --git a/tests/pure_queue.py b/tests/pure_queue.py index 559fe9f..4e709db 100644 --- a/tests/pure_queue.py +++ b/tests/pure_queue.py @@ -17,7 +17,7 @@ def test_empty_creation(): @pytest.mark.parametrize("items", ([], [0, 1, 2], (0, 1, 2), (0.0, 1.0, 2.0), range(100))) def test_creation(items: PureQueueInitializer): - queue = PureQueue() + queue = PureQueue(items) assert len(queue) == len(items) @@ -76,4 +76,4 @@ def test_mixed_iteration(): results.append(number) if len(queue): queue.insert(queue.pop() * 2) - assert results == [2, 6, 8, 16] + assert results == [2, 6, 8, 32] From 7184ddb9b0b0b125be40c54c4d8f5c9205dacbe4 Mon Sep 17 00:00:00 2001 From: Michael Bradley Date: Fri, 10 Jan 2025 21:33:45 +1300 Subject: [PATCH 4/4] Fully implement PureQueue --- src/backing/containers/py_item.rs | 4 +- src/queue/paired.rs | 2 +- src/queue/pure.rs | 61 +++++++++++++++++++++++++++---- 3 files changed, 56 insertions(+), 11 deletions(-) diff --git a/src/backing/containers/py_item.rs b/src/backing/containers/py_item.rs index 9c7110c..81c38af 100644 --- a/src/backing/containers/py_item.rs +++ b/src/backing/containers/py_item.rs @@ -8,12 +8,12 @@ pub struct PyItem(Py); impl PyItem { /// Creates a new instance - fn new(item: Py) -> Self { + pub fn new(item: Py) -> Self { PyItem(item) } /// Retrieves the internal data - fn item(self) -> Py { + pub fn data(self) -> Py { self.0 } } diff --git a/src/queue/paired.rs b/src/queue/paired.rs index 69aaee4..5bb5f06 100644 --- a/src/queue/paired.rs +++ b/src/queue/paired.rs @@ -1,4 +1,3 @@ -/// A "paired" priority queue that links some data to a priority and supports duplicates, but not arbitrary deletions or weight updates use crate::backing::{ containers::Pair, pure::{BinaryHeap, PureBacking}, @@ -14,6 +13,7 @@ pub struct PairedQueue { backing: Box, f64>>>, } +/// A "paired" priority queue that links some data to a priority and supports duplicates, but not arbitrary deletions or priority updates #[pymethods] impl PairedQueue { /// Enables generic typing diff --git a/src/queue/pure.rs b/src/queue/pure.rs index d040ff8..f15a1c8 100644 --- a/src/queue/pure.rs +++ b/src/queue/pure.rs @@ -1,4 +1,8 @@ -use pyo3::{prelude::*, types::PyType}; +use pyo3::{ + exceptions::{PyIndexError, PyStopIteration, PyTypeError}, + prelude::*, + types::PyType, +}; use crate::backing::{ containers::PyItem, @@ -10,23 +14,64 @@ pub struct PureQueue { backing: Box>, } +/// A "pure" priority queue just contains the priorities without any linked data, and does not support arbitrary deletions or priority updates #[pymethods] impl PureQueue { - /// Enables generic typing - #[classmethod] - fn __class_getitem__(cls_: Bound<'_, PyType>, _key: Py) -> Bound<'_, PyType> { - cls_ - } - #[new] #[pyo3(signature = (items=None))] fn new(items: Option>) -> PyResult { if let Some(py_object) = items { - todo!() + Python::with_gil(|py| { + if let Ok(vec) = py_object.extract::>>(py) { + Ok(Self { + backing: Box::new(BinaryHeap::from_iter(vec.into_iter().map(PyItem::new))), + }) + } else { + Err(PyErr::new::("Items was not a sequence")) + } + }) } else { Ok(Self { backing: Box::new(BinaryHeap::new()), }) } } + + fn insert(mut self_: PyRefMut<'_, Self>, item: Py) { + self_.backing.add(PyItem::new(item)); + } + + // Below methods are identical to PairedQueue + // Normally I'd try to solve using a trait with default implementations but that doesn't seem to play nice with #[pymethods] + // (Although I shouldn't rule it out) + + /// Enables generic typing + #[classmethod] + fn __class_getitem__(cls_: Bound<'_, PyType>, _key: Py) -> Bound<'_, PyType> { + cls_ + } + + fn __len__(self_: PyRef<'_, Self>) -> usize { + self_.backing.len() + } + + fn __iter__(self_: PyRef<'_, Self>) -> PyRef<'_, Self> { + self_ + } + + fn __next__(mut self_: PyRefMut<'_, Self>) -> PyResult> { + if let Some(item) = self_.backing.pop() { + Ok(item.data()) + } else { + Err(PyErr::new::(())) + } + } + + fn pop(mut self_: PyRefMut<'_, Self>) -> PyResult> { + if let Some(item) = self_.backing.pop() { + Ok(item.data()) + } else { + Err(PyErr::new::(())) + } + } }