Comments (3)
Hi thanks for these points.
I think that taking Box rather than &'a MyTrait in general makes a better programming interface, eliminating lifetimes. Examples: Page::add_plot and ContinuousView::add both take references to traits. In fact I was forced to change the latter function to add(mut self, repr: Box) because of lifetime issues in my project (more specifically, I created the data in a loop. The old solution would require me to store the data in a Vec and then loop again over that Vec to plot the data). It remains, however, to change to Box in other functions.
I think this is a good suggestion. I personally prefer boxed trait objects, as you say it will remove the lifetimes.
I don't like that there are only modules in the root. I find it better when the most useful / frequently used items are exported in the project root. This point is easy; just re-export in root.
I don't think currently we are at a stage where we can stablalise API yet. In the long run, as you say it would be good to move really common structs and traits in to the top level namespace, or perhaps include a prelude
submodule for importing everything common. I feel however that a little inconvenience now is a minor setback while the API is changing.
Naming: It's rather confusing to have struct line::Line, struct line::Style and also trait style::Line. I think we should find some way to rename them. Especially try to not make the Lines unique only through the module paths. We could for example name them struct Line, struct LineStyle and trait LineStyleTrait.
I'm not personally keen on having Trait
in a Trait
's name, however we are open to suggestions on naming etc. We can keep this discussion open. Apart from this I like your suggestion.
Remove the Style traits. I don't see the need for a trait for every kind of Style. The only one that has two implementors is style::Line. However these two implementors are identical.
Perhaps e.g. the Line
trait could be merged in with the Line
struct. I'm sure common behaviour will appear in the future, perhaps with special types of lines but for now there is only one type of line.
Naming: There are some opportunities for shorter names. Some words are quite commonly shortened, such as s/Representation/Repr.
I am undecided. Text editors support autocomplete so it's not the typing that's the problem. Long names lead to long lines which are more difficult to read.
from plotlib.
I've been having a look into your first item, changing references to trait objects into owned trait objects (Box
) to see how that changes the API. The changes can be seen here:
master...mindriot101:boxed-traits
In addition, for ergonomics I included From<T>
impls for the boxed traits so the user does not have to create a box themselves (https://github.com/mindriot101/plotlib/blob/9673b037057107095432d759e691c95522b1a14c/src/histogram.rs#L210)
Some things I noticed:
- Using boxed traits moves the original structs to be owned by "parent" structs. This change in ownership may not be what the user wants. It might be worth considering implementing
Clone
for the more lightweight structs in case the user wants multiple plot elements which are identical. - There is definitely a reduction in the number of references and lifetimes flying around. This is linked to the point before but could be considered an additional bonus.
from plotlib.
I removed my previous comment, to write again. I implemented the suggestions related to styles in #29.
Your second comment:
- An alternative to the
From
approach, is to makeadd<T: ContinuousRepresentation>(...)
, and do the boxing itself. - Yeah, I think implementing Clone is fine.
from plotlib.
Related Issues (20)
- [SOLVED]Letter Frequency Counter Issue HOT 2
- Add ability to title plots
- Wrong example for scatterplot HOT 3
- Fix examples HOT 2
- Contour plots HOT 4
- Making ContinuousView struct field public
- Plotting to term is broken since 0.5.0 HOT 3
- Set color for Axis Labels
- Chrono DateTime values for X-Axis
- Plans to replace failure dependency? HOT 3
- legend doesn't work well in PointStyle of ContinuousView when saving as svg
- Grouped Bar Charts
- Background color
- How to add a legend? HOT 1
- Flipped CategoricalView HOT 1
- Show Examples rendered in Github
- Allow for logarithmic scales in `ContinuousView`
- Allow for polygons in `ContinuousView`
- Categorical view to text produces empty string HOT 1
- Dependency changes have not made it to crates.io since last version update in 2020
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from plotlib.