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

It is really hard to extend the api, is there any plan to modify the api for easier using? #516

Open
baiguoname opened this issue Nov 3, 2023 · 9 comments

Comments

@baiguoname
Copy link

baiguoname commented Nov 3, 2023

For example, if I need to make a plot element drawing a rectangle, in which show a text:

struct RectAndText {
    rect: Rectangle<(dt, f32)>,
    text: String,
}

impl<'a> PointCollection<'a, (dt, f32)> for &'a RectAndText {
    type Point = &'a (dt ,f32);
    type IntoIter = &'a [(dt, f32)];
    fn point_iter(self) -> Self::IntoIter {
        self.rect.point_iter()
    }
}

If I plot the two part in one block, I would implement the type like this:

impl<DB: DrawingBackend> Drawable<DB> for RectAndText {
    fn draw<I: Iterator<Item = plotters_backend::BackendCoord>>(
        &self,
        mut points: I,
        backend: &mut DB,
        parent_dim: (u32, u32),
    ) -> Result<(), DrawingErrorKind<DB::ErrorType>> {
        // self.rect.draw(points, backend, parent_dim)?;
        match (points.next(), points.next()) {
            (Some(a), Some(b)) => {
                let (mut a, mut b) = ((a.0.min(b.0), a.1.min(b.1)), (a.0.max(b.0), a.1.max(b.1)));
                backend.draw_rect(a, b, &self.rect.style, self.rect.style.filled)?;
                let posi = (((a.0 + b.0) / 2), ((a.1 + b.1) / 2));
                let text_style = TextStyle {
                    font: FontDesc::new(plotters_backend::FontFamily::Name("monoc"), (a.1 - b.1) as f64 / 2., FontStyle::Normal),
                    color: BackendColor { alpha: 1., rgb: (100, 100, 100) },
                    pos: Pos::new(HPos::Center, VPos::Center),
                };
                backend.draw_text(&self.text, &text_style, posi)
            }
            _ => Ok(()),
        }
    }
}

But the rect.style is private, so I can't do that.
If I plot the two parts separately:

impl<DB: DrawingBackend> Drawable<DB> for RectAndText {
    fn draw<I: Iterator<Item = plotters_backend::BackendCoord>>(
        &self,
        mut points: I,
        backend: &mut DB,
        parent_dim: (u32, u32),
    ) -> Result<(), DrawingErrorKind<DB::ErrorType>> {
        self.rect.draw(points, backend, parent_dim)?;
        match (points.next(), points.next()) {
            (Some(a), Some(b)) => {
                
            }
            _ => Ok(()),
        }
    }
}

Then the points is lost. I find no way to do it.
Then I have to do it in this way, which does not effectively utilize the components that are already in place:


struct RectAndText {
    points: [(dt, f32); 2],
    style: ShapeStyle,
    text: String,
}

impl<'a> PointCollection<'a, (dt, f32)> for &'a RectAndText {
    type Point = &'a (dt ,f32);
    type IntoIter = &'a [(dt, f32)];
    fn point_iter(self) -> Self::IntoIter {
        &self.points
    }
}

impl<DB: DrawingBackend> Drawable<DB> for RectAndText {
    fn draw<I: Iterator<Item = plotters_backend::BackendCoord>>(
        &self,
        mut points: I,
        backend: &mut DB,
        _: (u32, u32),
    ) -> Result<(), DrawingErrorKind<DB::ErrorType>> {
        match (points.next(), points.next()) {
            (Some(a), Some(b)) => {
                let (a, b) = ((a.0.min(b.0), a.1.min(b.1)), (a.0.max(b.0), a.1.max(b.1)));
                backend.draw_rect(a, b, &self.style, self.style.filled)?;
                let posi = (((a.0 + b.0) / 2), ((a.1 + b.1) / 2));
                let text_style = TextStyle {
                    font: FontDesc::new(plotters_backend::FontFamily::Name("monoc"), (a.1 - b.1) as f64 / 2., FontStyle::Normal),
                    color: BackendColor { alpha: 1., rgb: (100, 100, 100) },
                    pos: Pos::new(HPos::Center, VPos::Center),
                };
                backend.draw_text(&self.text, &text_style, posi)
            }
            _ => Ok(()),
        }
    }
}
@AliMMehr
Copy link

AliMMehr commented Nov 3, 2023

One idea could be to collect the points iterator into a vector and then you can make multiple iterators from the collected vector, but I don't know about the performance implications:

impl<DB: DrawingBackend> Drawable<DB> for RectAndText {
    fn draw<I: Iterator<Item = plotters_backend::BackendCoord>>(
        &self,
        mut points: I,
        backend: &mut DB,
        parent_dim: (u32, u32),
    ) -> Result<(), DrawingErrorKind<DB::ErrorType>> {

        let points: Vec<plotters_backend::BackendCoord> = points.collect(); // collect points into a vector.

        self.rect.draw(points.iter().cloned(), backend, parent_dim)?;   // points.iter().cloned() will make an iterator. I use Iterator::cloned() to convert Iterator<&(i32, i32)> to Iterator<(i32, i32)>

        let mut points = points.iter().cloned(); // Again make another iterator

        match (points.next(), points.next()) {
            (Some(a), Some(b)) => {
                todo!()
            }
            _ => Ok(()),
        }
    }
}

@akmalsoliev
Copy link

Agreeing with @alimm1995, at current moment, the api has too much boiler code. Even a simple line plot requires 26 lines of code, when I can switch to python and achieve that with 3 lines

import matplotlib.pyplot as plt
plt.plot(x, y)
plt.show()

@baiguoname
Copy link
Author

One idea could be to collect the points iterator into a vector and then you can make multiple iterators from the collected vector, but I don't know about the performance implications:

impl<DB: DrawingBackend> Drawable<DB> for RectAndText {
    fn draw<I: Iterator<Item = plotters_backend::BackendCoord>>(
        &self,
        mut points: I,
        backend: &mut DB,
        parent_dim: (u32, u32),
    ) -> Result<(), DrawingErrorKind<DB::ErrorType>> {

        let points: Vec<plotters_backend::BackendCoord> = points.collect(); // collect points into a vector.

        self.rect.draw(points.iter().cloned(), backend, parent_dim)?;   // points.iter().cloned() will make an iterator. I use Iterator::cloned() to convert Iterator<&(i32, i32)> to Iterator<(i32, i32)>

        let mut points = points.iter().cloned(); // Again make another iterator

        match (points.next(), points.next()) {
            (Some(a), Some(b)) => {
                todo!()
            }
            _ => Ok(()),
        }
    }
}

Thanks for your solution, That collecting a Vec on every points may have a performance issues, especially when there are numerous parts on a figure.

@baiguoname
Copy link
Author

Agreeing with @alimm1995, at current moment, the api has too much boiler code. Even a simple line plot requires 26 lines of code, when I can switch to python and achieve that with 3 lines

import matplotlib.pyplot as plt
plt.plot(x, y)
plt.show()

It needs someone to pack the package into a new crate, just like seborn to matplotlib. I thought this may not have much difficulty if the plotters API is more extensible. But it is not, and it seems the author has stopped maintaining this crate. Maybe it's time for someone to make a new one based on this crate. 

@AaronErhardt
Copy link
Member

it seems the author has stopped maintaining this crate. Maybe it's time for someone to make a new one based on this crate.

It's true that the original maintainer of plotters is inactive, but plotters is still maintained. It is lacking active development though and IMO the crate could use some major clean-ups under the hood, but so far nobody has stepped up. Help with this would be highly appreciated :)

@akmalsoliev
Copy link

Agreeing that there are some major overhauls that are required and wrapper would be great. There is currently a crate that uses Plotly in backend, however, for me, the biggest advantage of this crate is the ability to display plots in CLI and natively.

@baiguoname
Copy link
Author

baiguoname commented Nov 21, 2023

Agreeing that there are some major overhauls that are required and wrapper would be great. There is currently a crate that uses Plotly in backend, however, for me, the biggest advantage of this crate is the ability to display plots in CLI and natively.

Plotly also seems to be no longer maintained by the author. I made a pr adding the trace of Table, the author has no reaction. I found that all the recent prs are not responding.
When I need to plot a simple and nice looking figure, I often use Plotly. But if I need reactive plotting, I have to do it in plotters. The combination of plotters and iced is really fast.

@akmalsoliev
Copy link

Agreeing that there are some major overhauls that are required and wrapper would be great. There is currently a crate that uses Plotly in backend, however, for me, the biggest advantage of this crate is the ability to display plots in CLI and natively.

Plotly also seems to be no longer maintained by the author. I made a pr adding the trace of Table, the author has no reaction. I found that all the recent prs are not responding. When I need to plot a simple and nice looking figure, I often use Plotly. But if I need reactive plotting, I have to do it in plotters. The combination of plotters and iced is really fast.

It appears to be active, last commit 2 weeks ago https://github.com/plotters-rs/plotters

@DarthB
Copy link

DarthB commented Nov 21, 2023

I think changing an API is always hard as you need to get all stakeholders on board. It seems like many crates are extending plotters and we want them to switch to the new version too. I don't have an overview how easy it is to reach the maintainers of those crates, but major changes should be discussed with them as well.

I think moving the style information in the type that wraps your rectangle is good. You may conclude the fill properties from the state of your "logical" rectangle but this should be done only once before drawing. My first instinct would be to extend Drawable with a

fn collect_styles(&mut self); 

that may be called early in draw? But as a new user of plotters, I'm not confident to change APIs that are affecting so many other places. Maybe a good start would be to reduce the mentioned "talkiness" because that could be done with additions without changing too much of the underlying API as a starting contributer?

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

5 participants