Traces for Mesh3d, Image, ScatterMapbox

[JoelSjogren]

Hi, I have implemented some basic wrapping code for go.Mesh3D and go.Image as suggested in #49. I used @mrhrzg's scatter3d.rs as a starting point.

I am sending this pull request now to get some feedback. Should I do the following things to complete the added features? Anything else?

• add one example for each new trace type
• implement setters for all remaining parameters listed in the documentation
• fix all cargo warnings
• add entry in change log
• run test suite

Just before sending this I decided to also add some mapbox support, as suggested in #86. Specifically I started adding go.Scattermapbox. I am a little confused about layout.mapbox and layout.mapbox_style which seems to lack a documentation entry, and I have not been able to get layout.mapbox_style to work properly. I tried "stamen-watercolor" and "open-street-map" but they seem to have no effect. I guess it should be turned into an enum in the end, but this can't be the problem can it?

By the way I am using Evcxr.

Also by the way I think there is a fair bit of code duplication going on with parameters being repeated across different structs. Maybe this could be improved using some kind of rust macro? I guess the most difficult part of this might be to handle the documentation properly.

[mfreeborn]

Thanks for the PR. Yes, as a basic requirement, the code would need to be rustfmted with all the warnings resolved.

Adding the remaining setters, along with tests would be necessary, and then crediting yourself in the changelog would be essential. Scatter3D and Sankey are good examples to base the code structure on.

I agree, there is a lot of code duplication, but I haven't figured out a good way of resolving that at present...

[JoelSjogren]

May I ask what is the reason for using traits so much?

For example
pub fn x0<V: Into<NumOrString>>(mut self, x0: V) -> Box<Self> {
instead of just
pub fn x0(mut self, x0: NumOrString) -> Box<Self> {
and also

pub fn new<I, K, L>(x: I, y: K, z: L) -> Box<Self>
    where
        I: IntoIterator<Item = X>,
        K: IntoIterator<Item = Y>,
        L: IntoIterator<Item = Z>,
    {

instead of just

pub fn new(x: &[X], y: &[Y], z: &[Z]) -> Box<Self>
    {

or even

pub fn new(x: Vec<X>, y: Vec<Y>, z: Vec<Z>) -> Box<Self>
    {

[mfreeborn]

Mostly it was a decision taken before I started contributing, but I think it does make for a clean user API. For NumOrString, for example, the user doesn't need to worry particularly about NumOrString, they can just pass whatever type implements Into<NumOrString> for it:

let trace = Scatter::new().x0(10_u32);

// or...

let trace = Scatter::new().x0("something");

Without, it would look something like:

let trace = Scatter::new().x0(NumOrString::I(10_u32 as i64));

// or...

let trace = Scatter::new().x0(NumOrString::S("something".to_string()));

I think the former is much nicer.

[JoelSjogren]

Ah I see, so it is sort of dual to the use of #[serde(untagged)].

[JoelSjogren]

Hi, in the latest commit I have sketched a new trait for the Image trace, to allow input directly from the ndarray or image packages without the user having to convert these into Vec<Vec<PixelColor<U>>> manually. Do you think this would be sensible, based on what we said about traits before? If so I need to figure out how to serialize this trait properly. Maybe you could give some comments on how to do this, e.g. what parts of the Color trait to reuse.

[JoelSjogren]

It is pretty much all done now, except for that part about an ImageData trait.

[JoelSjogren]

That's some intense refactoring going on there :-)

[mfreeborn]

Yes :D sorry it took a while to get around to your PR, as you can see there was a lot of groundwork going on in the meantime. I'll just put one or two more finishing touches to this PR today and get it merged.

…

________________________________ From: Joel Sjögren ***@***.***> Sent: Wednesday, November 2, 2022 9:07:19 PM To: igiagkiozis/plotly ***@***.***> Cc: Michael Freeborn ***@***.***>; Comment ***@***.***> Subject: Re: [igiagkiozis/plotly] Traces for Mesh3d, Image, ScatterMapbox (PR #88) That's some intense refactoring going on there :-) — Reply to this email directly, view it on GitHub<#88 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AHSVKWCKKOAAD7MPM4C2JX3WGLJYPANCNFSM5XNQT4IQ>. You are receiving this because you commented.Message ID: ***@***.***>