Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Could make BoxPlot a Cow? #4

Open
simonrw opened this issue Apr 15, 2018 · 1 comment
Open

Could make BoxPlot a Cow? #4

simonrw opened this issue Apr 15, 2018 · 1 comment

Comments

@simonrw
Copy link
Collaborator

simonrw commented Apr 15, 2018

plotlib/src/boxplot.rs

Lines 57 to 60 in c732707

enum BoxData<'a> {
Owned(Vec<f64>),
Ref(&'a [f64]),
}

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
Copy link
Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants