Fully implement PureQueue

Also reduce queue item trait bound from Ord to just PartialOrd

src/backing/containers/mod.rs

@@ -0,0 +1,5 @@
+mod pair;
+mod py_item;
+
+pub use pair::Pair;
+pub use py_item::PyItem;

src/backing/containers/pair.rs

@@ -0,0 +1,33 @@
+use std::cmp::Ordering;
+
+/// Container to associate an item with a priority
+#[derive(Debug, Clone, Copy)]
+pub struct Pair<D: Clone, P: PartialOrd + Clone> {
+    data: D,
+    priority: P,
+}
+
+impl<D: Clone, P: PartialOrd + Clone> Pair<D, P> {
+    /// Creates a new instance
+    pub fn new(data: D, priority: P) -> Self {
+        Self { data, priority }
+    }
+
+    /// 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
+    }
+}
+
+impl<D: Clone, P: PartialOrd + Clone> PartialOrd for Pair<D, P> {
+    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
+        self.priority.partial_cmp(&other.priority)
+    }
+}
+
+impl<D: Clone, P: PartialOrd + Clone> PartialEq for Pair<D, P> {
+    fn eq(&self, other: &Self) -> bool {
+        self.priority == other.priority
+    }
+}

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<PyAny>);
+
+impl PyItem {
+    /// Creates a new instance
+    pub fn new(item: Py<PyAny>) -> Self {
+        PyItem(item)
+    }
+
+    /// Retrieves the internal data
+    pub fn data(self) -> Py<PyAny> {
+        self.0
+    }
+}
+
+impl PartialOrd for PyItem {
+    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
+        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))
+    }
+}

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<D: Clone + Send + Sync, P: Ord + Clone + Send + Sync>:
-    PureBacking<Item<D, P>>
+pub trait IndexedBacking<D: Clone + Send + Sync, P: PartialOrd + Clone + Send + Sync>:
+    PureBacking<Pair<D, P>>
 {
     /// Update an item's priority
     fn update(data: D, priority: P) -> Result<(), ()>;

src/backing/item.rs

@@ -1,46 +0,0 @@
-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<D: Clone, P: PartialOrd + Clone> {
-    data: D,
-    priority: P,
-}
-
-impl<D: Clone, P: PartialOrd + Clone> Item<D, P> {
-    /// Creates a new instance
-    pub fn new(data: D, priority: P) -> Self {
-        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
-    pub fn data(self) -> D {
-        self.data
-    }
-}
-
-// The relevant Ord implementations are based just on the priority
-impl<D: Clone, P: PartialOrd + Clone> Ord for Item<D, P> {
-    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<D: Clone, P: PartialOrd + Clone> PartialOrd for Item<D, P> {
-    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
-        self.priority.partial_cmp(&other.priority)
-    }
-}
-
-impl<D: Clone, P: PartialOrd + Clone> PartialEq for Item<D, P> {
-    fn eq(&self, other: &Self) -> bool {
-        self.priority == other.priority
-    }
-}
-
-impl<D: Clone, P: PartialOrd + Clone> Eq for Item<D, P> {}

src/backing/mod.rs

@@ -1,3 +1,3 @@
+pub mod containers;
 pub mod indexed;
-pub mod item;
 pub mod pure;

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<T: Ord + Clone + Send + Sync> {
-    data: Vec<T>,
-}
-
 /// 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<T: Ord + Clone + Send + Sync> BinaryHeap<T> {
+/// A binary min-heap backed by an array
+#[derive(Debug)]
+pub struct BinaryHeap<T: PartialOrd + Clone + Send + Sync> {
+    data: Vec<T>,
+}
+
+impl<T: PartialOrd + Clone + Send + Sync> BinaryHeap<T> {
     /// Instantiates a new (empty) binary heap
     pub fn new() -> Self {
         Self { data: vec![] }
@@ -108,7 +108,7 @@ impl<T: Ord + Clone + Send + Sync> BinaryHeap<T> {
     }
 }
 
-impl<T: Ord + Clone + Send + Sync> FromIterator<T> for BinaryHeap<T> {
+impl<T: PartialOrd + Clone + Send + Sync> FromIterator<T> for BinaryHeap<T> {
     fn from_iter<U: IntoIterator<Item = T>>(iter: U) -> Self {
         let mut this = Self {
             data: Vec::from_iter(iter),
@@ -120,7 +120,7 @@ impl<T: Ord + Clone + Send + Sync> FromIterator<T> for BinaryHeap<T> {
     }
 }
 
-impl<T: Ord + Clone + Send + Sync> PureBacking<T> for BinaryHeap<T> {
+impl<T: PartialOrd + Clone + Send + Sync> PureBacking<T> for BinaryHeap<T> {
     fn add(&mut self, item: T) {
         // Append item
         self.data.push(item);

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<T: Ord + Send + Sync>: Send + Sync {
+pub trait PureBacking<T: PartialOrd + Send + Sync>: Send + Sync {
     /// Places an item into the queue
     fn add(&mut self, item: T);
     /// Removes the item with minimum priority, if it exists

src/queue/paired.rs

@@ -1,6 +1,5 @@
-/// 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,11 +10,13 @@ use pyo3::{
 
 #[pyclass]
 pub struct PairedQueue {
-    backing: Box<dyn PureBacking<Item<Py<PyAny>, f64>>>,
+    backing: Box<dyn PureBacking<Pair<Py<PyAny>, 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
     #[classmethod]
     fn __class_getitem__(cls_: Bound<'_, PyType>, _key: Py<PyAny>) -> Bound<'_, PyType> {
         cls_
@@ -62,12 +63,13 @@ impl PairedQueue {
     }
 
     fn __setitem__(mut self_: PyRefMut<'_, Self>, key: Py<PyAny>, 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<Vec<Item<Py<PyAny>, f64>>> {
+    /// Tries to a Python object into a vector suitable for ingestion into the backing
+    fn from_any(object: &Bound<'py, PyAny>) -> PyResult<Vec<Pair<Py<PyAny>, f64>>> {
         if let Ok(vec) = object.extract::<Vec<(Py<PyAny>, f64)>>() {
             Ok(Self::from_vec(vec))
         } else {
@@ -87,24 +89,26 @@ impl<'py> PairedQueue {
         }
     }
 
-    fn from_vec(list: Vec<(Py<PyAny>, f64)>) -> Vec<Item<Py<PyAny>, f64>> {
+    /// Converts a vector of Python objects and priorities into a vector of items
+    fn from_vec(list: Vec<(Py<PyAny>, f64)>) -> Vec<Pair<Py<PyAny>, 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<Vec<Item<Py<PyAny>, f64>>> {
+    /// Converts a Python dictionary into a vector of items
+    fn from_dict(dict: &Bound<'py, PyDict>) -> PyResult<Vec<Pair<Py<PyAny>, f64>>> {
         if let Ok(items) = dict
             .into_iter()
             .map(|(data, priority)| match priority.extract::<f64>() {
-                Ok(value) => Ok(Item::new(data.unbind(), value)),
+                Ok(value) => Ok(Pair::new(data.unbind(), value)),
                 Err(err) => Err(err),
             })
             .collect::<Result<Vec<_>, _>>()
         {
             Ok(items)
         } else {
-            Err(PyErr::new::<PyTypeError, _>("Dict keys were not floats"))
+            Err(PyErr::new::<PyTypeError, _>("Dict keys were not numbers"))
         }
     }
 }

src/queue/pure.rs

@@ -1,12 +1,77 @@
-use pyo3::prelude::*;
+use pyo3::{
+    exceptions::{PyIndexError, PyStopIteration, PyTypeError},
+    prelude::*,
+    types::PyType,
+};
+
+use crate::backing::{
+    containers::PyItem,
+    pure::{BinaryHeap, PureBacking},
+};
 
 #[pyclass]
-pub struct PureQueue {}
+pub struct PureQueue {
+    backing: Box<dyn PureBacking<PyItem>>,
+}
 
+/// A "pure" priority queue just contains the priorities without any linked data, and does not support arbitrary deletions or priority updates
 #[pymethods]
 impl PureQueue {
     #[new]
-    fn new() -> Self {
-        Self {}
+    #[pyo3(signature = (items=None))]
+    fn new(items: Option<Py<PyAny>>) -> PyResult<Self> {
+        if let Some(py_object) = items {
+            Python::with_gil(|py| {
+                if let Ok(vec) = py_object.extract::<Vec<Py<PyAny>>>(py) {
+                    Ok(Self {
+                        backing: Box::new(BinaryHeap::from_iter(vec.into_iter().map(PyItem::new))),
+                    })
+                } else {
+                    Err(PyErr::new::<PyTypeError, _>("Items was not a sequence"))
+                }
+            })
+        } else {
+            Ok(Self {
+                backing: Box::new(BinaryHeap::new()),
+            })
+        }
+    }
+
+    fn insert(mut self_: PyRefMut<'_, Self>, item: Py<PyAny>) {
+        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<PyAny>) -> 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<Py<PyAny>> {
+        if let Some(item) = self_.backing.pop() {
+            Ok(item.data())
+        } else {
+            Err(PyErr::new::<PyStopIteration, _>(()))
+        }
+    }
+
+    fn pop(mut self_: PyRefMut<'_, Self>) -> PyResult<Py<PyAny>> {
+        if let Some(item) = self_.backing.pop() {
+            Ok(item.data())
+        } else {
+            Err(PyErr::new::<PyIndexError, _>(()))
+        }
     }
 }

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)
 
 

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]