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

More flexible plotting (histogram, box plot...) #467

Closed
niladic opened this issue Jun 5, 2021 · 8 comments
Closed

More flexible plotting (histogram, box plot...) #467

niladic opened this issue Jun 5, 2021 · 8 comments

Comments

@niladic
Copy link
Contributor

niladic commented Jun 5, 2021

I am impressed by how nice the plotting is in egui. It would be really useful to have more capabilities, like histograms or box plots.

My attempt

I was not at all familiar with the codebase, but I tried to create such plots. A quick way I could get it done was: modify the Points struct, include something like a hashmap from Value to a dyn Trait that knows how to draw an element. This dyn Trait would contain the actual values of, say, a given box. The dyn Trait is to provide maximum flexibility to the end user:

// The "drawing trait"
pub trait DataPoint {
    // v- here `self` would contain `Value` (in a clean impl, this is not particularly desirable)
    fn get_shapes(&self, transform: &ScreenTransform) -> Vec<Shape>;
}

pub struct Points {
...
    // v- as it is, it would not work, since `Value` does not impl `Hash`
    points: HashMap<Value, Box<dyn DataPoint>>,
...
}

Then at the call site (get_shapes in ItemPlot), one can fetch the correct drawing impl for a given Value and create a Vec<Shape>.

There are a few problems with this approach:

  • Value is not Hash nor Eq, this means hacking arround it
  • the drawing trait is pub (so end users can create their own drawings), this means putting pub on ScreenTransform and Bounds
  • a given Value is duplicated in memory, both in the Values struct and the hashmap

(I am not sharing code about what I describe because its a mess ;))

Some ideas for implementing this feature

Idea 1

Creating a new impl for PlotItem similar the existing Points (like I describe above).
Benefits: minimal code modification. Drawbacks: the ones I cited above.

Idea 2

Modifying Value or Values, so they carry more data and the way they can be drawn. I am thinking about something like

pub trait ValueWrapper {
    fn value(&self) -> Value;
    fn shapes(&self, transform: &ScreenTransform) -> Vec<Shape>;
}
struct BoxPlotValue { ... };
impl ValueWrapper for BoxPlotValue { ... };

// such that (obviously wrong, I am just trying to convey the idea)
pub(super) trait PlotItem<V: ValueWrapper> {
...
    fn series(&self) -> &V;
...
}
impl Plot {
...
    pub fn points<V: ValueWrapper>(mut self, mut points: PlotItem<V>) -> Self {
...
}

Benefits: this seems to be the "conceptually correct way" to do it. In the future it can potentially be used to implement some custom axis labels (categorical data,...). Drawback: I have no idea where it leads to, there are probably some object safety issues. This might change a lot of code.


I don't know if such feature is in scope for the library. If it is, I would be glad to take a stab at it. Since this is the first time I play with eguis codebase, I am probably missing something. I am sure there are other, better ways to implement it, happy to discuss if someone thinks of one.

@EmbersArc
Copy link
Contributor

More additions to the plotting library are very much welcome from my side!

I'm not sure if adding this to Values or Points would be the right approach though. It might make sense to add a whole new kind of plot even. The current plot is very much focused on drawing X-Y data.

But maybe you can implement something that is analogous to the current Plot, that is made specifically for box and bar plots/histograms?

@niladic
Copy link
Contributor Author

niladic commented Jun 6, 2021

Thanks for the answer. Glad to hear this is not out of scope!

Just to see if I understand correctly, you are talking about doing another Widget, right?

To be specific, my use case for histograms is something like that https://github.com/38/plotters-doc-data/blob/master/normal-dist2.png (curve + histo). And same with boxes: curve + box. That's why I thought about piggybacking on the existing Plot. Kind of another layer like Points.

I will try to come up with some code with your suggestion, so we can discuss on something more concrete.

@EmbersArc
Copy link
Contributor

Ah, I see. Yes the curve+histo and curve+box use case mean that we shouldn't discard the idea of adding it to Plot right away. So maybe give it a try after all.

But I'd recommend not going with Value(s) for this since it's really only meant for XY data. You could argue that histograms like the one in your screenshot are basically just XY data as well. But as you recognized, we may want to support strings for categories (are they sometimes called bins?) as well. We should think about what kind of data the constructor of box and bar plots should take. I'm thinking something like HashSet or even BTreeSet (Doesn't require Eq) may be a good fit for the categories and then just series of Y values for all the other data (there are like 5 different series for box plots I think, if you include the whiskers).

I think we should be able to make the PlotItem trait more general to support box and bar plot as well. I've already changed it a bit in https://github.com/EmbersArc/egui/tree/more-plot-items for #468 just so that you're aware.

@niladic
Copy link
Contributor Author

niladic commented Jun 19, 2021

I did a draft in #499 for the specific plots I mentioned. I think I have found something simple, in line with your suggestion of making PlotItem more general. Basically removing references to Values in the methods of PlotItem. It is far from perfect, especially the HoverElement that owns a full Vec<Shape>. But then using references and lifetimes here kind of become hairy quick, it might not even be a problem in practice. If you see ways to improve it, I am open to suggestions. At one point, when the API is good enough, I think it would be great to open PlotItem so users can create their own shapes.

@akhilman
Copy link

Perhaps egui may implement backend for plotters.

@niladic
Copy link
Contributor Author

niladic commented Jul 4, 2021

Thanks for the suggestion @akhilman. I actually looked at plotters before opening this PR, but I did not see how to get back the interactivity in a simple way (zoom, hover, highlight, etc.).

@Bromeon
Copy link
Contributor

Bromeon commented Jan 16, 2022

I've implemented this (with your help) in #863, this issue can be closed.

@emilk
Copy link
Owner

emilk commented Jan 17, 2022

Closed by #863

@emilk emilk closed this as completed Jan 17, 2022
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