jturner314 / ndarray-npy

.npy and .npz file format support for ndarray
https://docs.rs/ndarray-npy
Apache License 2.0
56 stars 18 forks source link

Consider replacing Ext traits with functions #24

Closed matklad closed 5 years ago

matklad commented 5 years ago

Hi! I've tried using this crate today, and it works great!

However, it was slightly difficult to figure out due to the use of extension traits. As each trait has only one method, and is implemented on a single type, perhaps just providing

fn read_npy(impl Read) -> Result<ArrayBase<...>>
fn write_npy(impl Read, array: ArrayBase<...>) -> Result<()>

would give a simpler API? It also matches std::fs::read_to_string and np.save style

jturner314 commented 5 years ago

This is a good idea. I think a good API would be this:

pub fn read_npy<P, T>(path: P) -> Result<T, ReadNpyError>
where
    P: AsRef<std::path::Path>,
    T: ReadNpyExt,
{
    // ...
}

pub fn write_npy<P, T>(path: P, array: T) -> Result<(), WriteNpyError>
where
    P: AsRef<std::path::Path>,
    T: WriteNpyExt,
{
    // ...
}

(or maybe use ArrayBase instead of T) to match std::fs::read and std::fs::write.

I'll need to work on the error handling to make this work cleanly (since the version of ReadNpyError on master doesn't have an Io variant).