Traces for Mesh3d, Image, ScatterMapbox by JoelSjogren · Pull Request #88 · plotly/plotly.rs

JoelSjogren · 2022-05-31T13:10:30Z

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 · 2022-05-31T15:35:15Z

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 · 2022-06-02T11:08:04Z

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 · 2022-06-02T12:53:34Z

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 · 2022-06-02T13:16:43Z

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

JoelSjogren · 2022-06-10T06:47:33Z

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 · 2022-06-12T21:41:06Z

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

JoelSjogren · 2022-11-02T21:07:08Z

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

mfreeborn · 2022-11-03T08:16:38Z

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: ***@***.***>

* Refactor examples and readme (#113) * update main readme * update plotly_kaleido readme * update readme * typo * add readme * Refactor examples and readme * update changelog * fix CI workflow * update workflows * tpyo * formatting * fix workflows * update workflows * update readme * update readme * update url * final tweaks * Remove 'static lifetime requirement for `Plot.to_inline_html()` (#115) * remove 'static lifetime requirement * update changelog * add `legend_group_title` to existing traces (#110) * feat: add legendgrouptitle too all existing traces Fixes #109 * update changelog Co-authored-by: Michael Freeborn <michaelfreeborn1@gmail.com> * Traces for Mesh3d, Image, ScatterMapbox (#88) * Add basic Mesh3D trace functionality * Add basic Image trace functionality * Add basic ScatterMapbox functionality * Fix compilation errors due to merge * Copy some setters * Fill in some more Mesh3D setters * Complete the Mesh3D setters * Complete the Image setters * Sketch idea of ImageData trait * Complete the ScatterMapbox setters * Add tests for Mesh3D * Fix cargo warnings * Make greater use of IntoIterator trait * Add tests for Image * Add tests for ScatterMapbox * Fix compilation errors * Insert _ in setter names * Run rustfmt and add line breaks to documentation * Add jupyter lab examples for Mesh3D, Image, ScatterMapbox * Update CHANGELOG * Remove setter assertions * refactoring * fix tests * formatting * add ImageData trait * add Image trace examples * add mesh3d example * rustfmt * add map example * tweak zoom data type * rustfmt * clippy * update workflows Co-authored-by: Michael Freeborn <michaelfreeborn1@gmail.com> * add plotly_image to features list * v0.8.2 Co-authored-by: Kai Howelmeyer <kai@hoewelmeyer.eu> Co-authored-by: Joel Sjögren <joelsjogren.wii@gmail.com>

* Refactor examples and readme (plotly#113) * update main readme * update plotly_kaleido readme * update readme * typo * add readme * Refactor examples and readme * update changelog * fix CI workflow * update workflows * tpyo * formatting * fix workflows * update workflows * update readme * update readme * update url * final tweaks * Remove 'static lifetime requirement for `Plot.to_inline_html()` (plotly#115) * remove 'static lifetime requirement * update changelog * add `legend_group_title` to existing traces (plotly#110) * feat: add legendgrouptitle too all existing traces Fixes plotly#109 * update changelog Co-authored-by: Michael Freeborn <michaelfreeborn1@gmail.com> * Traces for Mesh3d, Image, ScatterMapbox (plotly#88) * Add basic Mesh3D trace functionality * Add basic Image trace functionality * Add basic ScatterMapbox functionality * Fix compilation errors due to merge * Copy some setters * Fill in some more Mesh3D setters * Complete the Mesh3D setters * Complete the Image setters * Sketch idea of ImageData trait * Complete the ScatterMapbox setters * Add tests for Mesh3D * Fix cargo warnings * Make greater use of IntoIterator trait * Add tests for Image * Add tests for ScatterMapbox * Fix compilation errors * Insert _ in setter names * Run rustfmt and add line breaks to documentation * Add jupyter lab examples for Mesh3D, Image, ScatterMapbox * Update CHANGELOG * Remove setter assertions * refactoring * fix tests * formatting * add ImageData trait * add Image trace examples * add mesh3d example * rustfmt * add map example * tweak zoom data type * rustfmt * clippy * update workflows Co-authored-by: Michael Freeborn <michaelfreeborn1@gmail.com> * add plotly_image to features list * v0.8.2 Co-authored-by: Kai Howelmeyer <kai@hoewelmeyer.eu> Co-authored-by: Joel Sjögren <joelsjogren.wii@gmail.com>

JoelSjogren added 4 commits May 31, 2022 14:48

Add basic Mesh3D trace functionality

135b5ff

Add basic Image trace functionality

e44d118

Add basic ScatterMapbox functionality

41ca15e

Fix compilation errors due to merge

b8e5d83

Copy some setters

55542c3

JoelSjogren added 4 commits June 8, 2022 14:22

Fill in some more Mesh3D setters

5a9bfdb

Complete the Mesh3D setters

2bee7d0

Complete the Image setters

50d8658

Sketch idea of ImageData trait

34afab9

JoelSjogren added 11 commits June 10, 2022 09:49

Complete the ScatterMapbox setters

fd257f8

Add tests for Mesh3D

e2b1663

Fix cargo warnings

e460665

Make greater use of IntoIterator trait

c652b27

Add tests for Image

a470a5d

Add tests for ScatterMapbox

4546811

Fix compilation errors

b5672c9

Insert _ in setter names

dd8079d

Run rustfmt and add line breaks to documentation

f78e7ca

Add jupyter lab examples for Mesh3D, Image, ScatterMapbox

4658099

Update CHANGELOG

c1ee3a2

JoelSjogren and others added 4 commits June 13, 2022 23:17

Remove setter assertions

d64cb51

Merge remote-tracking branch 'upstream/dev' into mesh3d

b03b7c0

refactoring

679075b

fix tests

ddcee2b

formatting

b5aeba1

mfreeborn added 9 commits November 3, 2022 11:31

add ImageData trait

3ff3f02

add Image trace examples

a41a089

add mesh3d example

fdf2536

rustfmt

e0b5799

add map example

fbfbb12

tweak zoom data type

6a75a25

rustfmt

ae88023

clippy

2d55b2a

update workflows

94cdeb1

mfreeborn merged commit 086d012 into plotly:dev Nov 3, 2022

This was referenced Nov 3, 2022

various kinds of plots #49

Closed

Mapbox Support #86

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Traces for Mesh3d, Image, ScatterMapbox#88

Traces for Mesh3d, Image, ScatterMapbox#88
mfreeborn merged 34 commits intoplotly:devfrom
JoelSjogren:mesh3d

JoelSjogren commented May 31, 2022

Uh oh!

mfreeborn commented May 31, 2022

Uh oh!

JoelSjogren commented Jun 2, 2022

Uh oh!

mfreeborn commented Jun 2, 2022 •

edited

Loading

Uh oh!

JoelSjogren commented Jun 2, 2022

Uh oh!

JoelSjogren commented Jun 10, 2022

Uh oh!

JoelSjogren commented Jun 12, 2022

Uh oh!

JoelSjogren commented Nov 2, 2022

Uh oh!

mfreeborn commented Nov 3, 2022 via email

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

JoelSjogren commented May 31, 2022

Uh oh!

mfreeborn commented May 31, 2022

Uh oh!

JoelSjogren commented Jun 2, 2022

Uh oh!

mfreeborn commented Jun 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JoelSjogren commented Jun 2, 2022

Uh oh!

JoelSjogren commented Jun 10, 2022

Uh oh!

JoelSjogren commented Jun 12, 2022

Uh oh!

JoelSjogren commented Nov 2, 2022

Uh oh!

mfreeborn commented Nov 3, 2022 via email

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mfreeborn commented Jun 2, 2022 •

edited

Loading