Clean up the Task and TaskData APIs (#24)

Not much changes here, but cleanup includes
 - adding `__repr__`
 - always passing timestamps as `datetime` / `DateTime<Utc>`
 - Annotations are immutable
 - tests for everything

src/replica.rs

@@ -47,7 +47,7 @@ impl Replica {
         let task = self
             .0
             .create_task(Uuid::parse_str(&uuid)?, ops.as_mut())
-            .map(Task)?;
+            .map(Task::from)?;
         Ok(task)
     }
 
@@ -57,7 +57,7 @@ impl Replica {
             .0
             .all_tasks()?
             .into_iter()
-            .map(|(key, value)| (key.to_string(), Task(value)))
+            .map(|(key, value)| (key.to_string(), value.into()))
             .collect())
     }
 
@@ -66,7 +66,7 @@ impl Replica {
             .0
             .all_task_data()?
             .into_iter()
-            .map(|(key, value)| (key.to_string(), TaskData(value)))
+            .map(|(key, value)| (key.to_string(), TaskData::from(value)))
             .collect())
     }
     /// Get a list of all uuids for tasks in the replica.
@@ -101,14 +101,14 @@ impl Replica {
         Ok(self
             .0
             .get_task(Uuid::parse_str(&uuid).unwrap())
-            .map(|opt| opt.map(Task))?)
+            .map(|opt| opt.map(Task::from))?)
     }
 
     pub fn get_task_data(&mut self, uuid: String) -> anyhow::Result<Option<TaskData>> {
         Ok(self
             .0
             .get_task_data(Uuid::parse_str(&uuid)?)
-            .map(|opt| opt.map(TaskData))?)
+            .map(|opt| opt.map(TaskData::from))?)
     }
 
     /// Sync with a server crated from `ServerConfig::Local`.

src/task/annotation.rs

@@ -1,36 +1,48 @@
-use chrono::DateTime;
+use chrono::{DateTime, Utc};
 use pyo3::prelude::*;
 use taskchampion::Annotation as TCAnnotation;
-#[pyclass]
+
+#[pyclass(frozen, eq)]
+#[derive(PartialEq, Eq)]
 /// An annotation for the task
-pub struct Annotation(pub(crate) TCAnnotation);
+pub struct Annotation(TCAnnotation);
 
 #[pymethods]
 impl Annotation {
     #[new]
-    pub fn new() -> Self {
-        Annotation(TCAnnotation {
-            entry: DateTime::default(),
-            description: String::new(),
-        })
+    pub fn new(entry: DateTime<Utc>, description: String) -> Self {
+        Annotation(TCAnnotation { entry, description })
     }
-    #[getter]
-    pub fn entry(&self) -> String {
-        self.0.entry.to_rfc3339()
+
+    pub fn __repr__(&self) -> String {
+        format!("{:?}", self.as_ref())
     }
 
-    #[setter]
-    pub fn set_entry(&mut self, time: String) {
-        self.0.entry = DateTime::parse_from_rfc3339(&time).unwrap().into()
+    #[getter]
+    pub fn entry(&self) -> DateTime<Utc> {
+        self.0.entry
     }
 
     #[getter]
     pub fn description(&self) -> String {
         self.0.description.clone()
     }
+}
+
+impl AsRef<TCAnnotation> for Annotation {
+    fn as_ref(&self) -> &TCAnnotation {
+        &self.0
+    }
+}
+
+impl From<TCAnnotation> for Annotation {
+    fn from(value: TCAnnotation) -> Self {
+        Annotation(value)
+    }
+}
 
-    #[setter]
-    pub fn set_description(&mut self, description: String) {
-        self.0.description = description
+impl From<Annotation> for TCAnnotation {
+    fn from(value: Annotation) -> Self {
+        value.0
     }
 }

src/task/data.rs

@@ -3,7 +3,7 @@ use pyo3::{exceptions::PyValueError, prelude::*};
 use taskchampion::{TaskData as TCTaskData, Uuid};
 
 #[pyclass]
-pub struct TaskData(pub(crate) TCTaskData);
+pub struct TaskData(TCTaskData);
 
 #[pymethods]
 impl TaskData {
@@ -13,7 +13,10 @@ impl TaskData {
         Ok(TaskData(TCTaskData::create(u, ops.as_mut())))
     }
 
-    #[getter(uuid)]
+    pub fn __repr__(&self) -> String {
+        format!("{:?}", self.0)
+    }
+
     pub fn get_uuid(&self) -> String {
         self.0.get_uuid().into()
     }
@@ -35,3 +38,21 @@ impl TaskData {
         self.0.delete(ops.as_mut());
     }
 }
+
+impl From<TCTaskData> for TaskData {
+    fn from(value: TCTaskData) -> Self {
+        TaskData(value)
+    }
+}
+
+impl From<TaskData> for TCTaskData {
+    fn from(value: TaskData) -> Self {
+        value.0
+    }
+}
+
+impl AsRef<TCTaskData> for TaskData {
+    fn as_ref(&self) -> &TCTaskData {
+        &self.0
+    }
+}

src/task/tag.rs

@@ -1,7 +1,8 @@
 use pyo3::{exceptions::PyValueError, prelude::*};
 use taskchampion::Tag as TCTag;
 
-#[pyclass]
+#[pyclass(frozen, eq, hash)]
+#[derive(PartialEq, Eq, Hash)]
 pub struct Tag(TCTag);
 
 #[pymethods]

src/task/task.rs

@@ -4,40 +4,41 @@ use chrono::{DateTime, Utc};
 use pyo3::prelude::*;
 use taskchampion::{Task as TCTask, Uuid};
 
-// TODO: actually create a front-facing user class, instead of this data blob
-#[pyclass]
-pub struct Task(pub(crate) TCTask);
-
-unsafe impl Send for Task {}
+// TODO: This type can be send once https://github.com/GothenburgBitFactory/taskchampion/pull/514
+// is available.
+#[pyclass(unsendable)]
+/// A TaskChampion Task.
+///
+/// This type is not Send, so it cannot be used from any thread but the one where it was created.
+pub struct Task(TCTask);
 
+#[pymethods]
 impl Task {
-    fn to_datetime(s: Option<String>) -> anyhow::Result<Option<DateTime<Utc>>> {
-        s.map(|time| Ok(DateTime::parse_from_rfc3339(&time)?.with_timezone(&chrono::Utc)))
-            .transpose()
+    fn __repr__(&self) -> String {
+        format!("{:?}", self.as_ref())
     }
-}
 
-#[pymethods]
-impl Task {
     #[allow(clippy::wrong_self_convention)]
     pub fn into_task_data(&self) -> TaskData {
-        TaskData(self.0.clone().into_task_data())
+        self.0.clone().into_task_data().into()
     }
+
     /// Get a tasks UUID
     ///
     /// Returns:
     ///     str: UUID of a task
-    // TODO: possibly determine if it's possible to turn this from/into python's UUID instead
     pub fn get_uuid(&self) -> String {
         self.0.get_uuid().to_string()
     }
+
     /// Get a task's status
     /// Returns:
     ///     Status: Status subtype
     pub fn get_status(&self) -> Status {
         self.0.get_status().into()
     }
 
+    /// Get a task's description
     pub fn get_description(&self) -> String {
         self.0.get_description().to_string()
     }
@@ -47,7 +48,6 @@ impl Task {
     /// Returns:
     ///     str: RFC3339 timestamp
     ///     None: No timestamp
-    // Attempt to convert this into a python datetime later on
     pub fn get_entry(&self) -> Option<DateTime<Utc>> {
         self.0.get_entry()
     }
@@ -68,6 +68,7 @@ impl Task {
     pub fn get_wait(&self) -> Option<DateTime<Utc>> {
         self.0.get_wait()
     }
+
     /// Check if the task is waiting
     ///
     /// Returns:
@@ -83,20 +84,23 @@ impl Task {
     pub fn is_active(&self) -> bool {
         self.0.is_active()
     }
+
     /// Check if the task is blocked
     ///
     /// Returns:
     ///     bool: if the task is blocked
     pub fn is_blocked(&self) -> bool {
         self.0.is_blocked()
     }
+
     /// Check if the task is blocking
     ///
     /// Returns:
     ///     bool: if the task is blocking
     pub fn is_blocking(&self) -> bool {
         self.0.is_blocking()
     }
+
     /// Check if the task has a tag
     ///
     /// Returns:
@@ -112,12 +116,13 @@ impl Task {
     pub fn get_tags(&self) -> Vec<Tag> {
         self.0.get_tags().map(Tag::from).collect()
     }
+
     /// Get task annotations
     ///
     /// Returns:
     ///     list[Annotation]: list of task annotations
     pub fn get_annotations(&self) -> Vec<Annotation> {
-        self.0.get_annotations().map(Annotation).collect()
+        self.0.get_annotations().map(Annotation::from).collect()
     }
 
     /// Get a task UDA
@@ -133,12 +138,10 @@ impl Task {
         self.0.get_uda(namespace, key)
     }
 
-    // TODO: this signature is ugly and confising, possibly replace this with a struct in the
-    // actual code
     /// get all the task's UDAs
     ///
     /// Returns:
-    ///     Uh oh, ew?
+    ///    List of tuples ((namespace, key), value)
     pub fn get_udas(&self) -> Vec<((&str, &str), &str)> {
         self.0.get_udas().collect()
     }
@@ -198,21 +201,30 @@ impl Task {
     }
 
     #[pyo3(signature=(entry, ops))]
-    pub fn set_entry(&mut self, entry: Option<String>, ops: &mut Operations) -> anyhow::Result<()> {
-        let timestamp = Self::to_datetime(entry)?;
-        Ok(self.0.set_entry(timestamp, ops.as_mut())?)
+    pub fn set_entry(
+        &mut self,
+        entry: Option<DateTime<Utc>>,
+        ops: &mut Operations,
+    ) -> anyhow::Result<()> {
+        Ok(self.0.set_entry(entry, ops.as_mut())?)
     }
 
     #[pyo3(signature=(wait, ops))]
-    pub fn set_wait(&mut self, wait: Option<String>, ops: &mut Operations) -> anyhow::Result<()> {
-        let timestamp = Self::to_datetime(wait)?;
-        Ok(self.0.set_wait(timestamp, ops.as_mut())?)
+    pub fn set_wait(
+        &mut self,
+        wait: Option<DateTime<Utc>>,
+        ops: &mut Operations,
+    ) -> anyhow::Result<()> {
+        Ok(self.0.set_wait(wait, ops.as_mut())?)
     }
 
     #[pyo3(signature=(modified, ops))]
-    pub fn set_modified(&mut self, modified: String, ops: &mut Operations) -> anyhow::Result<()> {
-        let timestamp = DateTime::parse_from_rfc3339(&modified)?.with_timezone(&chrono::Utc);
-        Ok(self.0.set_modified(timestamp, ops.as_mut())?)
+    pub fn set_modified(
+        &mut self,
+        modified: DateTime<Utc>,
+        ops: &mut Operations,
+    ) -> anyhow::Result<()> {
+        Ok(self.0.set_modified(modified, ops.as_mut())?)
     }
 
     #[pyo3(signature=(property, value, ops))]
@@ -245,13 +257,15 @@ impl Task {
         Ok(self.0.remove_tag(tag.as_ref(), ops.as_mut())?)
     }
 
-    pub fn add_annotation(&mut self, ann: &Annotation, ops: &mut Operations) -> anyhow::Result<()> {
-        // Create an owned annotation
-        let mut annotation = Annotation::new();
-        annotation.set_entry(ann.entry());
-        annotation.set_description(ann.description());
-
-        Ok(self.0.add_annotation(annotation.0, ops.as_mut())?)
+    pub fn add_annotation(
+        &mut self,
+        annotation: &Annotation,
+        ops: &mut Operations,
+    ) -> anyhow::Result<()> {
+        // Create an owned annotation (TODO: not needed once
+        // https://github.com/GothenburgBitFactory/taskchampion/pull/517 is available)
+        let annotation = Annotation::new(annotation.entry(), annotation.description());
+        Ok(self.0.add_annotation(annotation.into(), ops.as_mut())?)
     }
 
     pub fn remove_annotation(
@@ -313,3 +327,21 @@ impl Task {
         Ok(self.0.remove_dependency(dep_uuid, ops.as_mut())?)
     }
 }
+
+impl AsRef<TCTask> for Task {
+    fn as_ref(&self) -> &TCTask {
+        &self.0
+    }
+}
+
+impl From<TCTask> for Task {
+    fn from(value: TCTask) -> Self {
+        Task(value)
+    }
+}
+
+impl From<Task> for TCTask {
+    fn from(value: Task) -> Self {
+        value.0
+    }
+}

taskchampion.pyi

@@ -101,9 +101,9 @@ class Task:
     def set_status(self, status: "Status", ops: "Operations"): ...
     def set_description(self, description: str, ops: "Operations"): ...
     def set_priority(self, priority: str, ops: "Operations"): ...
-    def set_entry(self, entry: Optional[str], ops: "Operations"): ...
-    def set_wait(self, wait: Optional[str], ops: "Operations"): ...
-    def set_modified(self, modified: Optional[str], ops: "Operations"): ...
+    def set_entry(self, entry: Optional[datetime], ops: "Operations"): ...
+    def set_wait(self, wait: Optional[datetime], ops: "Operations"): ...
+    def set_modified(self, modified: datetime, ops: "Operations"): ...
     def set_value(self, property: str, value: Optional[str], ops: "Operations"): ...
     def start(self, ops: "Operations"): ...
     def stop(self, ops: "Operations"): ...
@@ -130,10 +130,10 @@ class WorkingSet:
     def __next__(self) -> tuple[int, str]: ...
 
 class Annotation:
-    entry: str
+    entry: datetime
     description: str
 
-    def __init__(self) -> None: ...
+    def __init__(self, entry: datetime, description: str) -> "Annotation": ...
 
 class DependencyMap:
     def dependencies(self, dep_of: str) -> list[str]: ...