Repository: extendr/extendr (rust)

Issue:
Document how to create a new matrix
At present there is no documentation that indicates how one can create a matrix. The example here is not clear. 

This request is for documentation and examples for how to make an extendr matrix. At present, matrices seem to be the most challenging extendr object to work with. 

https://github.com/extendr/extendr/blob/bae73e70ffa75f29c5e87719a66614134414a8cd/extendr-api/src/wrapper/matrix.rs#L7

Pull request:
Rmatrix docs
* Document `RArray::new_matrix()` (closes #463)
* Add a new `CollectRMatrix` trait that can be used to collect arbitrary iterables into an R matrix
* Includes tests for this new trait

Patch:
diff --git a/extendr-api/src/robj/into_robj.rs b/extendr-api/src/robj/into_robj.rs
index fda3b24de..8f8ce8ce4 100644
--- a/extendr-api/src/robj/into_robj.rs
+++ b/extendr-api/src/robj/into_robj.rs
@@ -477,6 +477,43 @@ pub trait RobjItertools: Iterator {
         assert!(vec.iter().size_hint() == (vec.len(), Some(vec.len())));
         vec.into_iter().collect_robj()
     }
+
+    /// Collects an iterable into an [`RArray`].
+    /// The iterable must yield items column by column (aka Fortan order)
+    ///
+    /// # Arguments
+    ///
+    /// * `dims` - an array containing the length of each dimension
+    fn collect_rarray<'a, const LEN: usize>(
+        self,
+        dims: [usize; LEN],
+    ) -> Result<RArray<Self::Item, [usize; LEN]>>
+    where
+        Self: Iterator,
+        Self: Sized,
+        Self::Item: ToVectorValue,
+        Robj: AsTypedSlice<'a, Self::Item>,
+        Self::Item: 'a,
+    {
+        let vector = self.collect_robj();
+        let prod = dims.iter().product::<usize>();
+        if prod != vector.len() {
+            return Err(Error::Other(format!(
+                "The vector length ({}) does not match the length implied by the dimensions ({})",
+                vector.len(),
+                prod
+            )));
+        }
+        let mut robj =
+            vector.set_attrib(wrapper::symbol::dim_symbol(), dims.iter().collect_robj())?;
+        let data = robj
+            .as_typed_slice_mut()
+            .ok_or(Error::Other(
+                "Unknown error in converting to slice".to_string(),
+            ))?
+            .as_mut_ptr();
+        Ok(RArray::from_parts(robj, data, dims))
+    }
 }
 
 // Thanks to *pretzelhammer* on stackoverflow for this.
@@ -605,3 +642,52 @@ impl From<Vec<Rfloat>> for Robj {
         Doubles::from_values(val.into_iter()).into()
     }
 }
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    #[test]
+    fn test_collect_rarray_matrix() {
+        test! {
+            // Check that collect_rarray works the same as R's matrix() function
+            let rmat = (1i32..=16).collect_rarray([4, 4]);
+            assert!(rmat.is_ok());
+            assert_eq!(Robj::from(rmat), R!("matrix(1:16, nrow=4)").unwrap());
+        }
+    }
+
+    #[test]
+    fn test_collect_rarray_tensor() {
+        test! {
+            // Check that collect_rarray works the same as R's array() function
+            let rmat = (1i32..=16).collect_rarray([2, 4, 2]);
+            assert!(rmat.is_ok());
+            assert_eq!(Robj::from(rmat), R!("array(1:16, dim=c(2, 4, 2))").unwrap());
+        }
+    }
+
+    #[test]
+    fn test_collect_rarray_matrix_failure() {
+        test! {
+            // Check that collect_rarray fails when given an invalid shape
+            let rmat = (1i32..=16).collect_rarray([3, 3]);
+            assert!(rmat.is_err());
+            let msg = rmat.unwrap_err().to_string();
+            assert!(msg.contains("9"));
+            assert!(msg.contains("dimension"));
+        }
+    }
+
+    #[test]
+    fn test_collect_tensor_failure() {
+        test! {
+            // Check that collect_rarray fails when given an invalid shape
+            let rmat = (1i32..=16).collect_rarray([3, 3, 3]);
+            assert!(rmat.is_err());
+            let msg = rmat.unwrap_err().to_string();
+            assert!(msg.contains("27"));
+            assert!(msg.contains("dimension"));
+        }
+    }
+}
diff --git a/extendr-api/src/robj/mod.rs b/extendr-api/src/robj/mod.rs
index 2b3cb94cd..ca8dc0c0f 100644
--- a/extendr-api/src/robj/mod.rs
+++ b/extendr-api/src/robj/mod.rs
@@ -1,11 +1,11 @@
 //! R object handling.
 //!
-//! See. <https://cran.r-project.org/doc/manuals/R-exts.html>
+//! See. [Writing R Extensions](https://cran.r-project.org/doc/manuals/R-exts.html)
 //!
 //! Fundamental principals:
 //!
 //! * Any function that can break the protection mechanism is unsafe.
-//! * Users should be able to do almost everything without using libR_sys.
+//! * Users should be able to do almost everything without using `libR_sys`.
 //! * The interface should be friendly to R users without Rust experience.
 //!
 
diff --git a/extendr-api/src/wrapper/matrix.rs b/extendr-api/src/wrapper/matrix.rs
index 9dd20c8d2..50894e73e 100644
--- a/extendr-api/src/wrapper/matrix.rs
+++ b/extendr-api/src/wrapper/matrix.rs
@@ -126,7 +126,16 @@ where
     Robj: AsTypedSlice<'a, T>,
 {
     /// Create a new matrix wrapper.
-    /// Make a new column type.
+    ///
+    /// # Arguments
+    ///
+    /// * `nrows` - the number of rows the returned matrix will have
+    /// * `ncols` - the number of columns the returned matrix will have
+    /// * `f` - a function that will be called for each entry of the matrix in order to populate it with values.
+    ///     It must return a scalar value that can be converted to an R scalar, such as `u32`, `f64`, i.e. see [ToVectorValue].
+    ///     It accepts two arguments:
+    ///     * `r` - the current row of the entry we are creating
+    ///     * `c` - the current column of the entry we are creating
     pub fn new_matrix<F: Clone + FnMut(usize, usize) -> T>(
         nrows: usize,
         ncols: usize,
diff --git a/extendr-engine/src/lib.rs b/extendr-engine/src/lib.rs
index 72562ee4d..19dead126 100644
--- a/extendr-engine/src/lib.rs
+++ b/extendr-engine/src/lib.rs
@@ -1,17 +1,18 @@
-//! A sigleton instance of the R interpreter.
+//! A singleton instance of the R interpreter.
 //!
-//! Only call this from main() if you want to run stand-alone.
+//! Only call this from `main()` if you want to run stand-alone.
 //!
 //! Its principal use is for testing.
 //!
-//! See <https://github.com/wch/r-source/blob/trunk/src/unix/Rembedded.c>
+//! See [Rembedded.c](https://github.com/wch/r-source/blob/trunk/src/unix/Rembedded.c).
+//!
 
 use libR_sys::*;
 use std::os::raw;
 use std::sync::Once;
 
 // Generate mutable static strings.
-// Much more efficient than CString.
+// Much more efficient than `CString`.
 // Generates asciiz.
 macro_rules! cstr_mut {
     ($s: expr) => {
diff --git a/extendr-macros/src/extendr_module.rs b/extendr-macros/src/extendr_module.rs
index 688b1c29f..d6f003454 100644
--- a/extendr-macros/src/extendr_module.rs
+++ b/extendr-macros/src/extendr_module.rs
@@ -48,8 +48,8 @@ pub fn extendr_module(item: TokenStream) -> TokenStream {
             #( #implmetanames(&mut impls); )*
 
             // Extends functions and impls with the submodules metadata
-            #( functions.extend(#usemetanames().functions); )*
-            #( impls.extend(#usemetanames().impls); )*
+            #( functions.extend(#usenames::#usemetanames().functions); )*
+            #( impls.extend(#usenames::#usemetanames().impls); )*
 
             // Add this function to the list, but set hidden: true.
             functions.push(extendr_api::metadata::Func {
@@ -88,6 +88,7 @@ pub fn extendr_module(item: TokenStream) -> TokenStream {
         #[no_mangle]
         #[allow(non_snake_case)]
         pub extern "C" fn #wrap_module_metadata_name() -> extendr_api::SEXP {
+            use extendr_api::GetSexp;
             unsafe { extendr_api::Robj::from(#module_metadata_name()).get() }
         }
 
@@ -99,6 +100,7 @@ pub fn extendr_module(item: TokenStream) -> TokenStream {
         ) -> extendr_api::SEXP {
             unsafe {
                 use extendr_api::robj::*;
+                use extendr_api::GetSexp;
                 let robj = new_owned(use_symbols_sexp);
                 let use_symbols: bool = <bool>::from_robj(&robj).unwrap();
 


Test patch:
diff --git a/extendr-api/tests/extendr_module_tests.rs b/extendr-api/tests/extendr_module_tests.rs
new file mode 100644
index 000000000..3aeea81ba
--- /dev/null
+++ b/extendr-api/tests/extendr_module_tests.rs
@@ -0,0 +1,41 @@
+//! Tests for [`extendr_module`] procedural macro.
+//!
+use extendr_api::{extendr, extendr_module};
+
+mod root {
+    use super::*;
+
+    mod nested_module {
+        use super::*;
+
+        #[extendr]
+        fn dummy() {}
+
+        extendr_module! {
+            mod nested_module;
+            fn dummy;
+        }
+    }
+
+    #[extendr]
+    fn hello_dummy() {}
+
+    extendr_module! {
+        mod top_level;
+        use nested_module;
+        use adjacent_module;
+        fn hello_dummy;
+    }
+}
+
+mod adjacent_module {
+    use super::*;
+
+    #[extendr]
+    fn foo() {}
+
+    extendr_module! {
+        mod adjacent_module;
+        fn foo;
+    }
+}
diff --git a/tests/extendrtests/src/rust/src/lib.rs b/tests/extendrtests/src/rust/src/lib.rs
index 94a8072db..bccc77b91 100644
--- a/tests/extendrtests/src/rust/src/lib.rs
+++ b/tests/extendrtests/src/rust/src/lib.rs
@@ -1,7 +1,6 @@
 use extendr_api::{graphics::*, prelude::*};
 
 mod submodule;
-use submodule::*;
 
 mod ndarray;
 use ndarray::*;
@@ -170,7 +169,7 @@ fn check_default(#[default = "NULL"] x: Robj) -> bool {
 }
 
 // Weird behavior of parameter descriptions:
-// first passes tests as is, second -- only in backqutoes.
+// first passes tests as is, second -- only in backquotes.
 /// Test whether `_arg` parameters are treated correctly in R
 /// Executes \code{`_x` - `_y`}
 /// @param _x an integer scalar, ignored