Repository: bluss/rust-ndarray
Pull request #256 (2016-12-24 14:05:19)

ndarray::Array macro array![]
This allows for Array and RcArray creation without specifying the dimensionality using macros.
On a sidenote, this also allows for compile time flattening of arrays, I've implemented a version of this [here](https://play.rust-lang.org/?gist=4317bd5029ff890669341b16861e5b0e&version=nightly&backtrace=0), I don't know if doing something similar to this is more efficient then what is being done now in rust-ndarray but maybe you find it interesting.

bluss: Hi, this is an interesting direction. I'd skip RcArray at all initially (it can always be done by calling .into_shared() at the end anyway).

We can do even better: We can create an array from a `vec![]` directly (no copying, and elements created mostly in place). We'd like to continue using fixed size array literals for compile time size checks, while using a vec in the outer layer. With something like that I think a new macro is really paying for itself.
bluss: Note that macros should use full paths to items it uses. for example it should use `$crate::arr1` instead of arr1.
Pireax: Could you expand on the 'We can do even better part'? Creating a multidimensional array using `vec`![[1], [2]]` for example will create a Vec with the elements `[1]` and `[2]`, this isn't what we want right? If we use a macro like this and just call `vec!` with the flat representation we miss the dimension sizes and don't get the  compile time checks right?
bluss: Exactly, but `vec![[1], [2]]` can be used to create a two dimensional array. It needs implementing a function similar to `arr2` but using a vector by value instead of copying elements.
bluss: Ok, It has some complications in turning a (for example) `Vec<[f32; 2]>` into a `Vec<f32>`, but I think it should be possible.
bluss: I've added new `From` impls on the master branch that can be used by the macro. Please rebase your branch (no merges in PRs).
bluss: I'd go with no 0-dim case (does it need to be in the macro? `array![x]` should be a one dimensional array I think).
Pireax: Sure, I'll rebase and remove the 0-dim case, makes more sense if it is a one dimensional array.
Pireax: By the way, what about something like [this](https://play.rust-lang.org/?gist=3d075d3d025495af936eb276b8f30326&version=stable&backtrace=0)?
bluss: What part of that snippet, and why?
Pireax: The macros, not the struct, the struct is just as an example.
If the compiler optimizes the array instantion away this is a way to get the flat array, the dimensionality and the dimension sizes at compile time.
bluss: I see. It's nice since it requires less unsafe hackery, but with some extra work to compute the array sizes. The ignored array literal will not work though, not exactly like that at least — the expressions may even have side effects.

Such a macro can be implemented using Array::from_shape_vec_unchecked.
Pireax: Array::from_shape_vec_unchecked doesn't check if all the dimensions are consistent right? Using that function in the macro without the ignored array literal would allow for arrays like `[[1, 2], [3, 4, 5]]` to pass.
bluss: Yes, it would need to check it some other way.
Pireax: I think this is a good compromise for now.
bluss: Using ArrayN::from is good, I don't see much drawbacks with it. If only I can get the corner cases of zero sizes right..
bluss: Ok, great, we have something that works well. To integrate, we need to fix

- [ ] Documentation for the new public API
  - Documentation is pretty important, since this will probably be a new major part of the face of the crate!
- [ ] Decide on name. `arr` seems to follow current naming convention (also mirror `vec`), but I'd think an extra round if not to use `array`. Extra thought: What name to use to be consistent with a possible array view counterpart?
- [ ] Update PR name and description to match the state of the change
Pireax: I think it'll be best to fix the naming first so we don't have to go through the docs twice.
On the naming, personally I think `arr` isn't descriptive at all, it doesn't really say anything. I think something like `ndarr` or more preferrably `ndarray` is more descriptive and clearer in its meaning. For the array view counterpart, aren't views kind of like slices? We could name it `ndslice` or alternatively `ndview`.
What do you think?
bluss: The most important part is just the doc comment for the macro itself, actually.

I'll think about the macro name.
Pireax: Something came up which I had to deal with, I'm currently fixing the docs (replacing all arr1, arr2, arr3 calls with the macro). There are two ways to go about it, one is to use an explicit main, macro_use and extern crate at all rustdoc code segments using the macro, or we could hide the main function, etc and use the macro implicitly. What do you think would be best?
bluss: My general policy is to show everything. Maybe show the extern crate line, but not the main function? I don't think all docs need to be updated though. I've also posted a compiling PR with another name. I haven't had much time.
Pireax: I personally found only showing `#[macro_use] extern crate ndarray` a bit odd as it implicitly states the code is top level while that wouldn't  compile. I've taken the approach used in the examples in the rust documentation book and hidden both the main function and the extern crate.
bluss: My initial comment about documentation was to emphasize that the macro needed documentation itself. I don't agree with the overall doc changes, it would be better to not include them. We'll show all the code, and pick a few strategic places where we use the macro in the docs.
Pireax: Ok, so what do you want me to do to get this integrated?
bluss: I'm sorry for the complications.

- The macro, with its doc comment and the text itself are what I'd want to integrate
bluss: See also bluss/rust-ndarray/pull/260
Pireax: Looking at that, it seems you have everything, are the still bugs or something since it's open?
bluss: I think it's unbuggy. The problematic code is all in Array::from (2d and 3d case). I was in fact considering to drop the 3D constructor, but I guess it can just stick around if there are no proven bugs with it.

I didn't have much time either, I cleaned up the PR some day since this one didn't pass tests then, but then you came back. It doesn't matter much to the code which PR goes in, but preferably yours so that your contribution becomes realized.
Pireax: Alright, I will update this PR tomorrow! If the Array::from becomes a problem we can always temporarily switch back to using arr2 / arr3 in the macro.
bluss: arr2, arr3 are already updated to use Array::from. It should be better code this way. `array![]` will move elements into place (just like `vec![]`) and arr2/arr3 use clone from a slice. But with Array::from they clone using `[_]::to_vec()`, which uses `.extend_from_slice()`, which has a specialization for `Copy` elements, so it should be good... (See that's simple to reason about :stuck_out_tongue:)

Patch:
diff --git a/src/free_functions.rs b/src/free_functions.rs
index 7952d07c..0bcad608 100644
--- a/src/free_functions.rs
+++ b/src/free_functions.rs
@@ -11,6 +11,38 @@ use std::mem::{size_of, forget};
 
 use imp_prelude::*;
 
+/// Easily create an [**`Array`**](type.Array.html) of up to 3 dimensions.
+/// 
+/// ```
+/// #[macro_use(array)]
+/// extern crate ndarray;
+///
+/// fn main() {
+///     let a1 = array![1, 2, 3, 4];
+///
+///     let a2 = array![[1, 2], [3, 4]];
+/// 
+///     let a3 = array![[[1, 2], [3, 4]], 
+///                     [[5, 6], [7, 8]]];
+///
+///     assert_eq!(a1.shape(), &[4]);
+///     assert_eq!(a2.shape(), &[2, 2]);
+///     assert_eq!(a3.shape(), &[2, 2, 2]);
+/// }
+/// ```
+#[macro_export]
+macro_rules! array {
+    ($([$([$($x:expr),*]),*]),*) => {{
+        $crate::Array3::from(vec![$([$([$($x,)*],)*],)*])
+    }};
+    ($([$($x:expr),*]),*) => {{
+        $crate::Array2::from(vec![$([$($x,)*],)*])
+    }};
+    ($($x:expr),*) => {{
+        $crate::Array::from_vec(vec![$($x,)*])
+    }};
+}
+
 /// Create a zero-dimensional array with the element `x`.
 pub fn arr0<A>(x: A) -> Array0<A>
 {


Test patch:
diff --git a/tests/array.rs b/tests/array.rs
index 07d21aeb..072d9cd5 100644
--- a/tests/array.rs
+++ b/tests/array.rs
@@ -1126,3 +1126,14 @@ fn test_array_clone_same_view() {
     let b = a.clone();
     assert_eq!(a, b);
 }
+
+#[test]
+fn array_macros() {
+    // array
+    let a1 = array![1, 2, 3];
+    assert_eq!(a1, arr1(&[1, 2, 3]));
+    let a2 = array![[1, 2], [3, 4], [5, 6]];
+    assert_eq!(a2, arr2(&[[1, 2], [3, 4], [5, 6]]));
+    let a3 = array![[[1, 2], [3, 4]], [[5, 6], [7, 8]]];
+    assert_eq!(a3, arr3(&[[[1, 2], [3, 4]], [[5, 6], [7, 8]]]));
+}
\ No newline at end of file