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

Add Table trace. #165

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Add Table trace. #165

wants to merge 8 commits into from

Conversation

baiguoname
Copy link

#148
This issue mentioned the trace.

@Nnamdi-sys
Copy link

Really great to see a Table Trace in the pipeline. Would also love to see a Pie Chart Trace if possible. Would love to contribute this, but don't have the requisite skill to do it yet.

@baiguoname
Copy link
Author

Really great to see a Table Trace in the pipeline. Would also love to see a Pie Chart Trace if possible. Would love to contribute this, but don't have the requisite skill to do it yet.

This is also first time I make a pr in plotly, after this pr is merged, I would like to make a similar Pie Chart pr.

@Nnamdi-sys
Copy link

Really great to see a Table Trace in the pipeline. Would also love to see a Pie Chart Trace if possible. Would love to contribute this, but don't have the requisite skill to do it yet.

This is also first time I make a pr in plotly, after this pr is merged, I would like to make a similar Pie Chart pr.

Hi @igiagkiozis, would be great to have this pull request approved in your next release. I tried it out and it works quite well. I also find it to be a very useful feature.

@kylemello
Copy link

@baiguoname I'm glad someone started work on this!
I do think it would make more sense if Table::new() takes a header and a cells struct, unless I'm missing something (I am a rust noob). I think in your code currently there is no way to change the header or cells style without creating a new instance of a header or cell with the same values you used in Table::new() and altering the header field in the struct with Table.header(). It makes more sense to me to create the cell or header beforehand with the values and styles you want and pass it into the table when you're happy with it. I hope that makes sense.
I've made a fork of your repo and am happy to submit a pr to it :) Lmk what you think. Heres the link to my fork: https://github.com/kylemello/plotly

@baiguoname
Copy link
Author

@baiguoname I'm glad someone started work on this! I do think it would make more sense if Table::new() takes a header and a cells struct, unless I'm missing something (I am a rust noob). I think in your code currently there is no way to change the header or cells style without creating a new instance of a header or cell with the same values you used in Table::new() and altering the header field in the struct with Table.header(). It makes more sense to me to create the cell or header beforehand with the values and styles you want and pass it into the table when you're happy with it. I hope that makes sense. I've made a fork of your repo and am happy to submit a pr to it :) Lmk what you think. Heres the link to my fork: https://github.com/kylemello/plotly

Thanks very much, I think it's great. Excuse me, I'm still new to github, what should I do next? Should I close this commit, and you pull a new pr to this crate?

@kylemello
Copy link

kylemello commented Jan 9, 2024

Thanks very much, I think it's great. Excuse me, I'm still new to github, what should I do next? Should I close this commit, and you pull a new pr to this crate?

Sorry for the late reply, I swear github never notifies me of anything.
No, I don't think so... I think me submitting a pr onto your branch will do just fine. I think once you merge it you'll be set. I haven't done a pull request on a pull request on GitHub yet so I'm just guessing here lol. Just opened it, here's the link: baiguoname#1

]
);
let mut plot = Plot::new();
plot.add_trace(table_trace);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@baiguoname There is a typo here. This should be trace

Tables now take a header and cells struct
@naijim
Copy link

naijim commented May 15, 2024

@baiguoname hi, is this feature still able to be merged? Really looking forward to see it.

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

Successfully merging this pull request may close these issues.

None yet

5 participants