From 5d5ed5be0b2e716f2a1b477625c1a7747c17db3d Mon Sep 17 00:00:00 2001
From: Andreas Longva <andreas.b.longva@gmail.com>
Date: Mon, 25 Jan 2021 17:12:46 +0100
Subject: [PATCH] Various minor doc and comment fixes

---
 nalgebra-sparse/src/cs.rs                     |  1 -
 nalgebra-sparse/src/csc.rs                    |  2 +-
 nalgebra-sparse/src/csr.rs                    |  2 +-
 nalgebra-sparse/src/factorization/cholesky.rs |  2 --
 nalgebra-sparse/src/ops/serial/pattern.rs     | 15 ++++++++++++++-
 nalgebra-sparse/src/pattern.rs                |  5 -----
 6 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/nalgebra-sparse/src/cs.rs b/nalgebra-sparse/src/cs.rs
index e9137aa5..f3c0a46d 100644
--- a/nalgebra-sparse/src/cs.rs
+++ b/nalgebra-sparse/src/cs.rs
@@ -197,7 +197,6 @@ impl<T> CsMatrix<T> {
 }
 
 impl<T: Scalar + One> CsMatrix<T> {
-    /// TODO
     #[inline]
     pub fn identity(n: usize) -> Self {
         let offsets: Vec<_> = (0 ..= n).collect();
diff --git a/nalgebra-sparse/src/csc.rs b/nalgebra-sparse/src/csc.rs
index f7a6b71b..8b5d0589 100644
--- a/nalgebra-sparse/src/csc.rs
+++ b/nalgebra-sparse/src/csc.rs
@@ -491,7 +491,7 @@ impl<T> CscMatrix<T>
 }
 
 impl<T: Scalar + One> CscMatrix<T> {
-    /// TODO
+    /// Constructs a CSC representation of the (square) `n x n` identity matrix.
     #[inline]
     pub fn identity(n: usize) -> Self {
         Self {
diff --git a/nalgebra-sparse/src/csr.rs b/nalgebra-sparse/src/csr.rs
index bdfb38e9..d1e9ab1f 100644
--- a/nalgebra-sparse/src/csr.rs
+++ b/nalgebra-sparse/src/csr.rs
@@ -491,7 +491,7 @@ where
 }
 
 impl<T: Scalar + One> CsrMatrix<T> {
-    /// TODO
+    /// Constructs a CSR representation of the (square) `n x n` identity matrix.
     #[inline]
     pub fn identity(n: usize) -> Self {
         Self {
diff --git a/nalgebra-sparse/src/factorization/cholesky.rs b/nalgebra-sparse/src/factorization/cholesky.rs
index 1c4f95a8..65a3c02f 100644
--- a/nalgebra-sparse/src/factorization/cholesky.rs
+++ b/nalgebra-sparse/src/factorization/cholesky.rs
@@ -140,8 +140,6 @@ impl<T: RealField> CscCholesky<T> {
     /// # Panics
     ///
     /// Panics if the matrix is not square.
-    ///
-    /// TODO: Take matrix by value or not?
     pub fn factor(matrix: &CscMatrix<T>) -> Result<Self, CholeskyError> {
         let symbolic = CscSymbolicCholesky::factor(matrix.pattern().clone());
         Self::factor_numerical(symbolic, matrix.values())
diff --git a/nalgebra-sparse/src/ops/serial/pattern.rs b/nalgebra-sparse/src/ops/serial/pattern.rs
index 276100f3..168a4d61 100644
--- a/nalgebra-sparse/src/ops/serial/pattern.rs
+++ b/nalgebra-sparse/src/ops/serial/pattern.rs
@@ -8,10 +8,13 @@ use std::iter;
 /// The patterns are assumed to have the same major and minor dimensions. In other words,
 /// both patterns `A` and `B` must both stem from the same kind of compressed matrix:
 /// CSR or CSC.
+///
+/// # Panics
+///
+/// Panics if the patterns don't have the same major and minor dimensions.
 pub fn spadd_pattern(a: &SparsityPattern,
                      b: &SparsityPattern) -> SparsityPattern
 {
-    // TODO: Proper error messages
     assert_eq!(a.major_dim(), b.major_dim(), "Patterns must have identical major dimensions.");
     assert_eq!(a.minor_dim(), b.minor_dim(), "Patterns must have identical minor dimensions.");
 
@@ -39,6 +42,11 @@ pub fn spadd_pattern(a: &SparsityPattern,
 ///
 /// Assumes that the sparsity patterns both represent CSC matrices, and the result is also
 /// represented as the sparsity pattern of a CSC matrix.
+///
+/// # Panics
+///
+/// Panics if the patterns, when interpreted as CSC patterns, are not compatible for
+/// matrix multiplication.
 pub fn spmm_csc_pattern(a: &SparsityPattern, b: &SparsityPattern) -> SparsityPattern {
     // Let C = A * B in CSC format. We note that
     //  C^T = B^T * A^T.
@@ -52,6 +60,11 @@ pub fn spmm_csc_pattern(a: &SparsityPattern, b: &SparsityPattern) -> SparsityPat
 ///
 /// Assumes that the sparsity patterns both represent CSR matrices, and the result is also
 /// represented as the sparsity pattern of a CSR matrix.
+///
+/// # Panics
+///
+/// Panics if the patterns, when interpreted as CSR patterns, are not compatible for
+/// matrix multiplication.
 pub fn spmm_csr_pattern(a: &SparsityPattern, b: &SparsityPattern) -> SparsityPattern {
     assert_eq!(a.minor_dim(), b.major_dim(), "a and b must have compatible dimensions");
 
diff --git a/nalgebra-sparse/src/pattern.rs b/nalgebra-sparse/src/pattern.rs
index 3a59a6e7..ee7edd42 100644
--- a/nalgebra-sparse/src/pattern.rs
+++ b/nalgebra-sparse/src/pattern.rs
@@ -118,11 +118,6 @@ impl SparsityPattern {
         major_offsets: Vec<usize>,
         minor_indices: Vec<usize>,
     ) -> Result<Self, SparsityPatternFormatError> {
-        // TODO: If these errors are *directly* propagated to errors from e.g.
-        // CSR construction, the error messages will be confusing to users,
-        // as the error messages refer to "major" and "minor" lanes, as opposed to
-        // rows and columns
-
         use SparsityPatternFormatError::*;
 
         if major_offsets.len() != major_dim + 1 {