search

milliams / plotlib

Data plotting library for Rust
https://plotlib.org
MIT License
462 stars 26 forks source link

Could make BoxPlot a Cow? #4

Open simonrw opened 6 years ago

simonrw commented 6 years ago

https://github.com/milliams/plotlib/blob/c732707b9bb29d51d42485d321df96d9d4d3d4b0/src/boxplot.rs#L57-L60

vs

pub enum Cow<'a, B> where
    B: 'a + ToOwned + ?Sized,  {
    Borrowed(&'a B),
    Owned(<B as ToOwned>::Owned),
}

This struct exactly matches the std::borrow::Cow type. Unless extra functionality is required, the stdlib type should probably be used.

milliams commented 6 years ago

Indeed it looks like API wise it would work as a replacement. However, I'm not sure if it makes sense semantically. The reason I was using the BoxData enum was to allow users of the BoxPlot representation to either move their data into the boxplot or simply have it referenced. It's not really intended to allow copy-on-write semantics.

It's still very undecided how the API for taking data from the user and putting it into some plotlib-internal data type should look. Currently BoxPlot is the only one that works like that and it was a bit of an experiment. We'll probably want consistent sematics across all the representations where possible.

Also, I haven't had much experience with Rust's managed pointers and COW types. Everything I've done so far has been simple ownership (with a ref references and Vecs) so it's possible I'm missing something.