Use PyResult for all return values to Python (#35)

In many cases, this performs a similar conversion as that supplied
automatically by pyo3, but it also allows raising ValueError for invalid
UUIDs instead of the default RuntimeError.

This also fixes the type of the `timestamp` property of
`Operation.Update`, which was missed when converting other datetimes to
use the `datetime`/`DateTime<Utc>` types.

* Temporarily use Poetry 1.x

.github/workflows/ci.yml

@@ -191,7 +191,8 @@ jobs:
       - uses: actions/setup-python@v5
         with:
           python-version: 3.9
-      - run: pip install poetry
+      # See #36 for upgrading to poetry 2.0.
+      - run: pip install 'poetry<2'
       - run: poetry install
       - run: poetry run pip install wheels-linux-x86_64/*
       - run: poetry run pytest
@@ -207,4 +208,4 @@ jobs:
           python-version: 3.9
       - run: pip install wheels-linux-x86_64/*
       - run: pip install mypy
-      - run: python -m mypy.stubtest taskchampion --ignore-missing-stub
+      - run: python -m mypy.stubtest taskchampion --ignore-missing-stub

.github/workflows/tests.yml

@@ -37,7 +37,8 @@ jobs:
         with:
           python-version: 3.9
 
-      - run: pip install poetry
+      # See #36 for upgrading to poetry 2.0.
+      - run: pip install 'poetry<2'
       - run: poetry install
       - run: poetry run maturin develop
       - run: poetry run pytest

Cargo.toml

@@ -14,7 +14,6 @@ crate-type = ["cdylib"]
 doc = false
 
 [dependencies]
-pyo3 = { version = "0.22.6", features = ["anyhow", "chrono"] }
+pyo3 = { version = "0.22.6", features = ["chrono"] }
 chrono = "*"
-anyhow = "*"
 taskchampion = { version = "=1.0.2" }

src/lib.rs

@@ -14,6 +14,8 @@ use operations::*;
 mod task;
 use task::{Annotation, Status, Tag, Task, TaskData};
 
+mod util;
+
 #[pymodule]
 fn taskchampion(m: &Bound<'_, PyModule>) -> PyResult<()> {
     m.add_class::<Status>()?;

src/operation.rs

@@ -1,8 +1,8 @@
-use chrono::DateTime;
+use crate::util::uuid2tc;
+use chrono::{DateTime, Utc};
 use pyo3::{exceptions::PyAttributeError, prelude::*};
-
 use std::collections::HashMap;
-use taskchampion::{Operation as TCOperation, Uuid};
+use taskchampion::Operation as TCOperation;
 
 #[pyclass]
 #[derive(PartialEq, Eq, Clone, Debug)]
@@ -17,17 +17,17 @@ pub struct Operation(pub(crate) TCOperation);
 impl Operation {
     #[allow(non_snake_case)]
     #[staticmethod]
-    pub fn Create(uuid: String) -> anyhow::Result<Operation> {
+    pub fn Create(uuid: String) -> PyResult<Operation> {
         Ok(Operation(TCOperation::Create {
-            uuid: Uuid::parse_str(&uuid)?,
+            uuid: uuid2tc(uuid)?,
         }))
     }
 
     #[allow(non_snake_case)]
     #[staticmethod]
-    pub fn Delete(uuid: String, old_task: HashMap<String, String>) -> anyhow::Result<Operation> {
+    pub fn Delete(uuid: String, old_task: HashMap<String, String>) -> PyResult<Operation> {
         Ok(Operation(TCOperation::Delete {
-            uuid: Uuid::parse_str(&uuid)?,
+            uuid: uuid2tc(uuid)?,
             old_task,
         }))
     }
@@ -38,16 +38,16 @@ impl Operation {
     pub fn Update(
         uuid: String,
         property: String,
-        timestamp: String,
+        timestamp: DateTime<Utc>,
         old_value: Option<String>,
         value: Option<String>,
-    ) -> anyhow::Result<Operation> {
+    ) -> PyResult<Operation> {
         Ok(Operation(TCOperation::Update {
-            uuid: Uuid::parse_str(&uuid)?,
+            uuid: uuid2tc(uuid)?,
             property,
             old_value,
             value,
-            timestamp: DateTime::parse_from_rfc3339(&timestamp).unwrap().into(),
+            timestamp,
         }))
     }
 
@@ -113,10 +113,10 @@ impl Operation {
     }
 
     #[getter(timestamp)]
-    pub fn get_timestamp(&self) -> PyResult<String> {
+    pub fn get_timestamp(&self) -> PyResult<DateTime<Utc>> {
         use TCOperation::*;
         match &self.0 {
-            Update { timestamp, .. } => Ok(timestamp.to_string()),
+            Update { timestamp, .. } => Ok(*timestamp),
             _ => Err(PyAttributeError::new_err(
                 "Variant does not have attribute 'timestamp'",
             )),

src/replica.rs

@@ -1,10 +1,10 @@
-use std::collections::HashMap;
-use std::rc::Rc;
-
 use crate::task::TaskData;
+use crate::util::{into_runtime_error, uuid2tc};
 use crate::{DependencyMap, Operations, Task, WorkingSet};
 use pyo3::prelude::*;
-use taskchampion::{Replica as TCReplica, ServerConfig, StorageConfig, Uuid};
+use std::collections::HashMap;
+use std::rc::Rc;
+use taskchampion::{Replica as TCReplica, ServerConfig, StorageConfig};
 
 #[pyclass(unsendable)]
 /// A replica represents an instance of a user's task data, providing an easy interface
@@ -24,64 +24,73 @@ impl Replica {
     ///     create_if_missing (bool): create the database if it does not exist
     /// Raises:
     ///     RuntimeError: if database does not exist, and create_if_missing is false
-    pub fn new_on_disk(path: String, create_if_missing: bool) -> anyhow::Result<Replica> {
+    pub fn new_on_disk(path: String, create_if_missing: bool) -> PyResult<Replica> {
         Ok(Replica(TCReplica::new(
             StorageConfig::OnDisk {
                 taskdb_dir: path.into(),
                 create_if_missing,
             }
-            .into_storage()?,
+            .into_storage()
+            .map_err(into_runtime_error)?,
         )))
     }
 
     #[staticmethod]
-    pub fn new_in_memory() -> anyhow::Result<Self> {
+    pub fn new_in_memory() -> PyResult<Self> {
         Ok(Replica(TCReplica::new(
-            StorageConfig::InMemory.into_storage()?,
+            StorageConfig::InMemory
+                .into_storage()
+                .map_err(into_runtime_error)?,
         )))
     }
 
     /// Create a new task
     /// The task must not already exist.
-    pub fn create_task(&mut self, uuid: String, ops: &mut Operations) -> anyhow::Result<Task> {
+    pub fn create_task(&mut self, uuid: String, ops: &mut Operations) -> PyResult<Task> {
         let task = self
             .0
-            .create_task(Uuid::parse_str(&uuid)?, ops.as_mut())
-            .map(Task::from)?;
+            .create_task(uuid2tc(uuid)?, ops.as_mut())
+            .map_err(into_runtime_error)?
+            .into();
         Ok(task)
     }
 
     /// Get a list of all tasks in the replica.
-    pub fn all_tasks(&mut self) -> anyhow::Result<HashMap<String, Task>> {
+    pub fn all_tasks(&mut self) -> PyResult<HashMap<String, Task>> {
         Ok(self
             .0
-            .all_tasks()?
+            .all_tasks()
+            .map_err(into_runtime_error)?
             .into_iter()
             .map(|(key, value)| (key.to_string(), value.into()))
             .collect())
     }
 
-    pub fn all_task_data(&mut self) -> anyhow::Result<HashMap<String, TaskData>> {
+    pub fn all_task_data(&mut self) -> PyResult<HashMap<String, TaskData>> {
         Ok(self
             .0
-            .all_task_data()?
+            .all_task_data()
+            .map_err(into_runtime_error)?
             .into_iter()
             .map(|(key, value)| (key.to_string(), TaskData::from(value)))
             .collect())
     }
     /// Get a list of all uuids for tasks in the replica.
-    pub fn all_task_uuids(&mut self) -> anyhow::Result<Vec<String>> {
+    pub fn all_task_uuids(&mut self) -> PyResult<Vec<String>> {
         Ok(self
             .0
             .all_task_uuids()
-            .map(|v| v.iter().map(|item| item.to_string()).collect())?)
+            .map_err(into_runtime_error)?
+            .iter()
+            .map(|item| item.to_string())
+            .collect())
     }
 
-    pub fn working_set(&mut self) -> anyhow::Result<WorkingSet> {
-        Ok(self.0.working_set().map(WorkingSet::from)?)
+    pub fn working_set(&mut self) -> PyResult<WorkingSet> {
+        Ok(self.0.working_set().map_err(into_runtime_error)?.into())
     }
 
-    pub fn dependency_map(&mut self, force: bool) -> anyhow::Result<DependencyMap> {
+    pub fn dependency_map(&mut self, force: bool) -> PyResult<DependencyMap> {
         // `Rc<T>` is not thread-safe, so we must get an owned copy of the data it contains.
         // Unfortunately, it cannot be cloned, so this is impossible (but both issues are fixed in
         // https://github.com/GothenburgBitFactory/taskchampion/pull/514).
@@ -91,37 +100,44 @@ impl Replica {
         // that TaskChampion does not modify a DependencyMap after creating it.
         //
         // This is a temporary hack, and should not be used in "real" code!
-        let dm = self.0.dependency_map(force)?;
+        let dm = self.0.dependency_map(force).map_err(into_runtime_error)?;
         // NOTE: this does not decrement the reference count and thus "leaks" the Rc.
         let dm_ptr = Rc::into_raw(dm);
         Ok(dm_ptr.into())
     }
 
-    pub fn get_task(&mut self, uuid: String) -> anyhow::Result<Option<Task>> {
+    pub fn get_task(&mut self, uuid: String) -> PyResult<Option<Task>> {
         Ok(self
             .0
-            .get_task(Uuid::parse_str(&uuid).unwrap())
-            .map(|opt| opt.map(Task::from))?)
+            .get_task(uuid2tc(uuid)?)
+            .map_err(into_runtime_error)?
+            .map(|t| t.into()))
     }
 
-    pub fn get_task_data(&mut self, uuid: String) -> anyhow::Result<Option<TaskData>> {
+    pub fn get_task_data(&mut self, uuid: String) -> PyResult<Option<TaskData>> {
         Ok(self
             .0
-            .get_task_data(Uuid::parse_str(&uuid)?)
-            .map(|opt| opt.map(TaskData::from))?)
+            .get_task_data(uuid2tc(uuid)?)
+            .map_err(into_runtime_error)?
+            .map(TaskData::from))
+    }
+
+    pub fn commit_operations(&mut self, ops: Operations) -> PyResult<()> {
+        self.0
+            .commit_operations(ops.into())
+            .map_err(into_runtime_error)
     }
 
     /// Sync with a server crated from `ServerConfig::Local`.
-    fn sync_to_local(&mut self, server_dir: String, avoid_snapshots: bool) -> anyhow::Result<()> {
+    fn sync_to_local(&mut self, server_dir: String, avoid_snapshots: bool) -> PyResult<()> {
         let mut server = ServerConfig::Local {
             server_dir: server_dir.into(),
         }
-        .into_server()?;
-        Ok(self.0.sync(&mut server, avoid_snapshots)?)
-    }
-
-    pub fn commit_operations(&mut self, ops: Operations) -> anyhow::Result<()> {
-        Ok(self.0.commit_operations(ops.into())?)
+        .into_server()
+        .map_err(into_runtime_error)?;
+        self.0
+            .sync(&mut server, avoid_snapshots)
+            .map_err(into_runtime_error)
     }
 
     /// Sync with a server created from `ServerConfig::Remote`.
@@ -131,14 +147,17 @@ impl Replica {
         client_id: String,
         encryption_secret: String,
         avoid_snapshots: bool,
-    ) -> anyhow::Result<()> {
+    ) -> PyResult<()> {
         let mut server = ServerConfig::Remote {
             url,
-            client_id: Uuid::parse_str(&client_id)?,
+            client_id: uuid2tc(client_id)?,
             encryption_secret: encryption_secret.into(),
         }
-        .into_server()?;
-        Ok(self.0.sync(&mut server, avoid_snapshots)?)
+        .into_server()
+        .map_err(into_runtime_error)?;
+        self.0
+            .sync(&mut server, avoid_snapshots)
+            .map_err(into_runtime_error)
     }
 
     /// Sync with a server created from `ServerConfig::Gcp`.
@@ -149,37 +168,48 @@ impl Replica {
         credential_path: Option<String>,
         encryption_secret: String,
         avoid_snapshots: bool,
-    ) -> anyhow::Result<()> {
+    ) -> PyResult<()> {
         let mut server = ServerConfig::Gcp {
             bucket,
             credential_path,
             encryption_secret: encryption_secret.into(),
         }
-        .into_server()?;
-        Ok(self.0.sync(&mut server, avoid_snapshots)?)
+        .into_server()
+        .map_err(into_runtime_error)?;
+        self.0
+            .sync(&mut server, avoid_snapshots)
+            .map_err(into_runtime_error)
     }
 
-    pub fn rebuild_working_set(&mut self, renumber: bool) -> anyhow::Result<()> {
-        Ok(self.0.rebuild_working_set(renumber)?)
+    pub fn rebuild_working_set(&mut self, renumber: bool) -> PyResult<()> {
+        self.0
+            .rebuild_working_set(renumber)
+            .map_err(into_runtime_error)
     }
 
-    pub fn num_local_operations(&mut self) -> anyhow::Result<usize> {
-        Ok(self.0.num_local_operations()?)
+    pub fn num_local_operations(&mut self) -> PyResult<usize> {
+        self.0.num_local_operations().map_err(into_runtime_error)
     }
 
-    pub fn num_undo_points(&mut self) -> anyhow::Result<usize> {
-        Ok(self.0.num_local_operations()?)
+    pub fn num_undo_points(&mut self) -> PyResult<usize> {
+        self.0.num_local_operations().map_err(into_runtime_error)
     }
 
-    pub fn get_undo_operations(&mut self) -> anyhow::Result<Operations> {
-        Ok(self.0.get_undo_operations()?.into())
+    pub fn get_undo_operations(&mut self) -> PyResult<Operations> {
+        Ok(self
+            .0
+            .get_undo_operations()
+            .map_err(into_runtime_error)?
+            .into())
     }
 
-    pub fn commit_reversed_operations(&mut self, operations: Operations) -> anyhow::Result<bool> {
-        Ok(self.0.commit_reversed_operations(operations.into())?)
+    pub fn commit_reversed_operations(&mut self, operations: Operations) -> PyResult<bool> {
+        self.0
+            .commit_reversed_operations(operations.into())
+            .map_err(into_runtime_error)
     }
 
-    pub fn expire_tasks(&mut self) -> anyhow::Result<()> {
-        Ok(self.0.expire_tasks()?)
+    pub fn expire_tasks(&mut self) -> PyResult<()> {
+        self.0.expire_tasks().map_err(into_runtime_error)
     }
 }