Repository: bluss/rust-ndarray
Pull request #115 (2016-03-03 14:48:28)

Updated serde serialization to `0.7`
No deserialization yet. But the output looks like the one produced by `rustc-serialize`. Check it with:

````
% cargo test serial --features "serialize_serde rustc-serialize" -- --nocapture
````

Also, I have a spurious commit due to updating my fork using github's pull requests. No idea how to fix that. :(

SuperFluffy: I should note that the `"Array"` key is not written (at least in `serde_json`) for this definition:

````
serializer.serialize_struct("Array",
    AVisitor {
        arr: self,
        state: 0,
})
````

I am not really sure why the key is dropped.
bluss: You have a stray merge in the commit history. It needs to be removed. You can for example rebase your branch on top of ndarray's master.
bluss: It looks like `serde_json` does not need to be a dependency.
bluss: Thanks for working on this. We need to support serialize for all array types, and deserialize for `DataOwned`-kinds of arrays I guess. I'd like to remove the dependency on serde_json; we shouldn't pull this in when it is not needed.
bluss: Ok this is good. We need to implement Serialize for all `ArrayBase` kinds (views too). Then it needs a documentation string like rustc-serialize has that mentions the crate feature.

It's ok to merge with just Serialization, but I'd prefer if Deserialization existed too.

The question is how we should handle serde instability. User that depend on us will break if we upgrade serde, and the reverse..

To solve the tests, we can make a separate internal crate for that. That's easier to configure to pull in crates only needed for testing.
SuperFluffy: Can we hide the `serde` serialization/deserialization behind a feature gate and make it work with nightly only? Next to keeping it as a feature one has to exlicitly opt into with `--features "serde"` I think this will be sufficient. Add a warning to the docs too.

It also just occured to me that we might want to consider actually integrating a `c` or `fortran` order flag now. Can this be done with the `ARRAY_FORMAT_VERSION`?

I will work on this towards the end of next week. Got to wrap up a few other projects.
bluss: np. Yes maybe we should support a different memory order, just to serialize faster. Right now it's using the logical order of the elements, so it's agnostic.
SuperFluffy: Are we rather interested in a simple but straightforward serialization, writing the underlying `Vec` directly into a list and providing the shape information with an extra field? Or do we prefer to encode the shape by storing the `Vec` as nested lists? Compare to how one could do it in `numpy`:

````python
In [0]: import json
In [1]: import numpy as np
In [2]: a = np.arange(12).reshape(2,3,2)

In [3]: a
Out[3]: 
array([[[ 0,  1],
        [ 2,  3],
        [ 4,  5]],

       [[ 6,  7],
        [ 8,  9],
        [10, 11]]])

In [4]: b = a.tolist()

In [5]: json.dumps(b)
Out[5]: '[[[0, 1], [2, 3], [4, 5]], [[6, 7], [8, 9], [10, 11]]]'
````
bluss: Shape up front is useful for several reasons, so we want that. I'd stick with the existing format. I don't see too much value in the nested format really? I guess it's valuable for pretty small data structures.
SuperFluffy: Yeah, I was thinking the same. It's great for eyeballing some data, but you don't want to do that once it gets too large. 
bluss: Json isn't even the most sensible target, ideally the user uses something like bincode I think.
SuperFluffy: Ok, looks like serialization and deserialization on `ArrayBase` is working now. All round-trip tests, where I have adapted those from `rustc-serialize`, are working now. The error handling is also similar to the one you use for `rustc-serialize`.

Please have a look at it and tell me what you think. I would be quite excited if this got merged.

Patch:
diff --git a/Cargo.toml b/Cargo.toml
index 008a4d45..d4dedda2 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -39,6 +39,14 @@ optional = true
 blas-sys = { version = "0.6.3", optional = true, default-features = false }
 matrixmultiply = { version = "0.1.8" }
 
+[dependencies.serde]
+version = "0.7"
+optional = true
+
+[dev-dependencies.serde_json]
+version = "0.7"
+
+
 [features]
 blas = ["blas-sys"]
 
diff --git a/src/arraydeserialize.rs b/src/arraydeserialize.rs
new file mode 100644
index 00000000..1e36dcbc
--- /dev/null
+++ b/src/arraydeserialize.rs
@@ -0,0 +1,124 @@
+// Copyright 2014-2016 bluss and ndarray developers.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+use serde::{self, Deserialize};
+
+use std::marker::PhantomData;
+
+use imp_prelude::*;
+
+use super::arraytraits::ARRAY_FORMAT_VERSION;
+
+struct ArrayVisitor<S,Di> {
+    _marker_a: PhantomData<S>,
+    _marker_b: PhantomData<Di>,
+}
+
+enum ArrayField {
+    Version,
+    Dim,
+    Data,
+}
+
+impl<S,Di> ArrayVisitor<S,Di> {
+    pub fn new() -> Self {
+        ArrayVisitor { _marker_a: PhantomData, _marker_b: PhantomData, }
+    }
+}
+
+impl<A, Di, S> Deserialize for ArrayBase<S, Di>
+    where A: Deserialize,
+          Di: Deserialize + Dimension,
+          S: DataOwned<Elem = A>
+{
+    fn deserialize<D>(deserializer: &mut D) -> Result<ArrayBase<S,Di>, D::Error>
+        where D: serde::de::Deserializer
+    {
+        static FIELDS: &'static [&'static str] = &["v", "dim", "data"];
+
+        deserializer.deserialize_struct("Array", FIELDS, ArrayVisitor::new())
+    }
+}
+
+impl serde::de::Deserialize for ArrayField {
+    fn deserialize<D>(deserializer: &mut D) -> Result<ArrayField, D::Error>
+        where D: serde::de::Deserializer
+    {
+        struct ArrayFieldVisitor;
+
+        impl serde::de::Visitor for ArrayFieldVisitor {
+            type Value = ArrayField;
+
+            fn visit_str<E>(&mut self, value: &str) -> Result<ArrayField, E>
+                where E: serde::de::Error
+            {
+                match value {
+                    "v" => Ok(ArrayField::Version),
+                    "data" => Ok(ArrayField::Data),
+                    "dim" => Ok(ArrayField::Dim),
+                    _ => Err(serde::de::Error::custom("expected v, data, or dim")),
+                }
+            }
+        }
+
+        deserializer.deserialize(ArrayFieldVisitor)
+    }
+}
+
+impl<A, Di, S> serde::de::Visitor for ArrayVisitor<S,Di>
+    where A: Deserialize,
+          Di: Deserialize + Dimension,
+          S: DataOwned<Elem = A>
+{
+    type Value = ArrayBase<S, Di>;
+
+    fn visit_map<V>(&mut self, mut visitor: V) -> Result<ArrayBase<S,Di>, V::Error>
+        where V: serde::de::MapVisitor,
+    {
+        let mut v: Option<u8> = None;
+        let mut data: Option<Vec<A>> = None;
+        let mut dim: Option<Di> = None;
+
+        loop {
+            match try!(visitor.visit_key()) {
+                Some(ArrayField::Version) => { v = Some(try!(visitor.visit_value())); },
+                Some(ArrayField::Data) => { data = Some(try!(visitor.visit_value())); },
+                Some(ArrayField::Dim) => { dim = Some(try!(visitor.visit_value())); },
+                None => { break; },
+            }
+        }
+
+        let v = match v {
+            Some(v) => v,
+            None => try!(visitor.missing_field("v")),
+        };
+
+        if v != ARRAY_FORMAT_VERSION {
+            try!(Err(serde::de::Error::custom(format!("unknown array version: {}", v))));
+        }
+
+        let data = match data {
+            Some(data) => data,
+            None => try!(visitor.missing_field("data")),
+        };
+
+        let dim = match dim {
+            Some(dim) => dim,
+            None => try!(visitor.missing_field("dim")),
+        };
+
+        if data.len() != dim.size() {
+            try!(Err(serde::de::Error::custom("data and dimension must match in size")));
+        }
+
+        try!(visitor.end());
+
+        unsafe {
+            Ok(ArrayBase::from_shape_vec_unchecked(dim, data))
+        }
+    }
+}
diff --git a/src/arrayserialize.rs b/src/arrayserialize.rs
index d4723a13..613bddca 100644
--- a/src/arrayserialize.rs
+++ b/src/arrayserialize.rs
@@ -6,62 +6,73 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 use serde::ser::impls::SeqIteratorVisitor;
-use serde::{self, Serialize, Deserialize};
+use serde::{self, Serialize};
 
-use super::{
-    Array,
-    Dimension,
-    Ix,
-    Elements,
-};
+use imp_prelude::*;
 
-struct AVisitor<'a, A: 'a, D: 'a> {
-    arr: &'a RcArray<A, D>,
+use super::arraytraits::ARRAY_FORMAT_VERSION;
+use super::Elements;
+
+struct AVisitor<'a, D: 'a, S: 'a>
+    where S: Data
+{
+    arr: &'a ArrayBase<S, D>,
     state: u32,
 }
 
-impl<A: Serialize, D: Serialize> Serialize for RcArray<A, D>
-    where D: Dimension
+impl<A, D, S> Serialize for ArrayBase<S, D>
+    where A: Serialize,
+          D: Dimension + Serialize,
+          S: DataOwned<Elem = A>
+
 {
-    fn serialize<S>(&self, serializer: &mut S) -> Result<(), S::Error>
-        where S: serde::Serializer
+    fn serialize<Se>(&self, serializer: &mut Se) -> Result<(), Se::Error>
+        where Se: serde::Serializer
     {
-        serializer.visit_named_map("Array",
-                                   AVisitor {
-                                       arr: self,
-                                       state: 0,
-                                   })
+        serializer.serialize_struct("Array",
+            AVisitor {
+                arr: self,
+                state: 0,
+        })
     }
 }
 
-impl<'a, A: Serialize, D: Serialize> Serialize for Elements<'a, A, D>
-    where D: Dimension
+impl<'a, A, D> Serialize for Elements<'a, A, D>
+    where A: Serialize,
+          D: Dimension + Serialize
 {
     fn serialize<S>(&self, serializer: &mut S) -> Result<(), S::Error>
         where S: serde::Serializer
     {
-        serializer.visit_seq(SeqIteratorVisitor::new(self.clone(), None))
+        serializer.serialize_seq(SeqIteratorVisitor::new(
+            self.clone(),
+            None,
+        ))
     }
 }
 
-impl<'a, A, D> serde::ser::MapVisitor for AVisitor<'a, A, D>
+impl<'a, A, D, S> serde::ser::MapVisitor for AVisitor<'a, D, S>
     where A: Serialize,
-          D: Serialize + Dimension
+          D: Serialize + Dimension,
+          S: DataOwned<Elem = A>
 {
-    fn visit<S>(&mut self, serializer: &mut S) -> Result<Option<()>, S::Error>
-        where S: serde::Serializer
+    fn visit<Se>(&mut self, serializer: &mut Se) -> Result<Option<()>, Se::Error>
+        where Se: serde::Serializer
     {
         match self.state {
             0 => {
-                self.state += 1;
-                Ok(Some(try!(serializer.visit_map_elt("shape", self.arr.dim()))))
-            }
+                self.state +=1;
+                Ok(Some(try!(serializer.serialize_map_elt("v", ARRAY_FORMAT_VERSION))))
+            },
             1 => {
                 self.state += 1;
-                Ok(Some(try!(serializer.visit_map_elt("data", self.arr.iter()))))
-            }
+                Ok(Some(try!(serializer.serialize_struct_elt("dim", self.arr.dim()))))
+            },
+            2 => {
+                self.state += 1;
+                Ok(Some(try!(serializer.serialize_struct_elt("data", self.arr.iter()))))
+            },
             _ => Ok(None),
         }
-
     }
 }
diff --git a/src/arraytraits.rs b/src/arraytraits.rs
index b357765c..64bbf9e5 100644
--- a/src/arraytraits.rs
+++ b/src/arraytraits.rs
@@ -208,10 +208,9 @@ unsafe impl<S, D> Send for ArrayBase<S, D>
     where S: Send + Data, D: Send
 { }
 
-
-#[cfg(feature = "rustc-serialize")]
+#[cfg(any(feature = "rustc-serialize", feature = "serde"))]
 // Use version number so we can add a packed format later.
-static ARRAY_FORMAT_VERSION: u8 = 1u8;
+pub const ARRAY_FORMAT_VERSION: u8 = 1u8;
 
 /// **Requires crate feature `"rustc-serialize"`**
 #[cfg(feature = "rustc-serialize")]
@@ -256,6 +255,7 @@ impl<A, S, D> Decodable for ArrayBase<S, D>
             let dim: D = try!(d.read_struct_field("dim", 1, |d| {
                 Decodable::decode(d)
             }));
+
             let elements = try!(
                 d.read_struct_field("data", 2, |d| {
                     d.read_seq(|d, len| {
diff --git a/src/lib.rs b/src/lib.rs
index 7a9a4d0e..167ba06f 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -114,6 +114,8 @@ pub use shape_builder::{ ShapeBuilder };
 mod arraytraits;
 #[cfg(feature = "serde")]
 mod arrayserialize;
+#[cfg(feature = "serde")]
+mod arraydeserialize;
 mod arrayformat;
 mod data_traits;
 


Test patch:
diff --git a/tests/serialize.rs b/tests/serialize.rs
index 1a7f5261..5dc076c8 100644
--- a/tests/serialize.rs
+++ b/tests/serialize.rs
@@ -6,45 +6,14 @@ extern crate ndarray;
 #[cfg(feature = "serde")]
 extern crate serde;
 
+#[cfg(feature = "serde")]
+extern crate serde_json;
+
 #[cfg(feature = "rustc-serialize")]
 use serialize::json;
 
 use ndarray::{arr0, arr1, arr2, RcArray, Ix, S, Si};
 
-#[cfg(feature = "serde")]
-#[test]
-fn test_serde()
-{
-    let mut out = std::io::stdout();
-    {
-        let a = arr0::<f32>(2.72);
-        println!("{:?}", a);
-        let v = serde::json::to_value(&a);
-        serde::json::to_writer(&mut out, &a);
-        /*
-        let serial = serde::json::encode(&a).unwrap();
-        println!("Encode {:?} => {:?}", a, serial);
-        */
-        //let res = json::decode::<RcArray<f32, _>>(&serial);
-        //println!("{:?}", res);
-        //assert_eq!(a, res.unwrap());
-    }
-
-    {
-        let a = arr2(&[[2.7, 1., 2.],
-                       [ 1., 0., 1.]]);
-        println!("{:?}", a);
-        serde::json::to_writer(&mut out, &a);
-        /*
-        let serial = serde::json::encode(&a).unwrap();
-        println!("{:?}", serial);
-        let res = json::decode::<RcArray<f32, _>>(&serial);
-        println!("{:?}", res);
-        assert_eq!(a, res.unwrap());
-        */
-    }
-}
-
 #[cfg(feature = "rustc-serialize")]
 #[test]
 fn serial_many_dim()
@@ -60,9 +29,8 @@ fn serial_many_dim()
 
     {
         let a = arr1::<f32>(&[2.72, 1., 2.]);
-        println!("{:?}", a);
         let serial = json::encode(&a).unwrap();
-        println!("{:?}", serial);
+        println!("Encode {:?} => {:?}", a, serial);
         let res = json::decode::<RcArray<f32, _>>(&serial);
         println!("{:?}", res);
         assert_eq!(a, res.unwrap());
@@ -70,9 +38,8 @@ fn serial_many_dim()
 
     {
         let a = arr2(&[[3., 1., 2.2], [3.1, 4., 7.]]);
-        println!("{:?}", a);
         let serial = json::encode(&a).unwrap();
-        println!("{:?}", serial);
+        println!("Encode {:?} => {:?}", a, serial);
         let res = json::decode::<RcArray<f32, _>>(&serial);
         println!("{:?}", res);
         assert_eq!(a, res.unwrap());
@@ -86,9 +53,8 @@ fn serial_many_dim()
         // Test a sliced array.
         let mut a = RcArray::linspace(0., 31., 32).reshape((2, 2, 2, 4));
         a.islice(&[Si(0, None, -1), S, S, Si(0, Some(2), 1)]);
-        println!("{:?}", a);
         let serial = json::encode(&a).unwrap();
-        println!("{:?}", serial);
+        println!("Encode {:?} => {:?}", a, serial);
         let res = json::decode::<RcArray<f32, _>>(&serial);
         println!("{:?}", res);
         assert_eq!(a, res.unwrap());
@@ -111,3 +77,66 @@ fn serial_wrong_count()
     println!("{:?}", arr);
     assert!(arr.is_err());
 }
+
+#[cfg(feature = "serde")]
+#[test]
+fn serial_many_dim_serde()
+{
+    {
+        let a = arr0::<f32>(2.72);
+        let serial = serde_json::to_string(&a).unwrap();
+        println!("Serde encode {:?} => {:?}", a, serial);
+        let res = serde_json::from_str::<RcArray<f32, _>>(&serial);
+        println!("{:?}", res);
+        assert_eq!(a, res.unwrap());
+    }
+
+    {
+        let a = arr1::<f32>(&[2.72, 1., 2.]);
+        let serial = serde_json::to_string(&a).unwrap();
+        println!("Serde encode {:?} => {:?}", a, serial);
+        let res = serde_json::from_str::<RcArray<f32, _>>(&serial);
+        println!("{:?}", res);
+        assert_eq!(a, res.unwrap());
+    }
+
+    {
+        let a = arr2(&[[3., 1., 2.2], [3.1, 4., 7.]]);
+        let serial = serde_json::to_string(&a).unwrap();
+        println!("Serde encode {:?} => {:?}", a, serial);
+        let res = serde_json::from_str::<RcArray<f32, _>>(&serial);
+        println!("{:?}", res);
+        assert_eq!(a, res.unwrap());
+        let text = r##"{"v":1,"dim":[2,3],"data":[3,1,2.2,3.1,4,7]}"##;
+        let b = serde_json::from_str::<RcArray<f32, (Ix, Ix)>>(text);
+        assert_eq!(a, b.unwrap());
+    }
+
+    {
+        // Test a sliced array.
+        let mut a = RcArray::linspace(0., 31., 32).reshape((2, 2, 2, 4));
+        a.islice(&[Si(0, None, -1), S, S, Si(0, Some(2), 1)]);
+        let serial = serde_json::to_string(&a).unwrap();
+        println!("Encode {:?} => {:?}", a, serial);
+        let res = serde_json::from_str::<RcArray<f32, _>>(&serial);
+        println!("{:?}", res);
+        assert_eq!(a, res.unwrap());
+    }
+}
+
+#[cfg(feature = "serde")]
+#[test]
+fn serial_wrong_count()
+{
+    // one element too few
+    let text = r##"{"v":1,"dim":[2,3],"data":[3,1,2.2,3.1,4]}"##;
+    let arr = serde_json::from_str::<RcArray<f32, (Ix, Ix)>>(text);
+    println!("{:?}", arr);
+    assert!(arr.is_err());
+
+    // future version
+    let text = r##"{"v":200,"dim":[2,3],"data":[3,1,2.2,3.1,4,7]}"##;
+    let arr = serde_json::from_str::<RcArray<f32, (Ix, Ix)>>(text);
+    println!("{:?}", arr);
+    assert!(arr.is_err());
+}