-
Notifications
You must be signed in to change notification settings - Fork 522
Support flexible x-axis for bars in BarChart #10675
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi! Thanks for opening this pull request.
Because this is your first time contributing to this repository, make sure you've read our Contributor Guide and Code of Conduct.
Thank you for your contribution! What's the status of your work, do you plan on finishing this up or is it ready for review yet? |
Hi grtlr, It's my first PR to rerun. So I make it a draft PR as requested. It's ready for review now except the failed RP label check. |
Thank you for the clarification! We'll put it onto our review queue! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for your contribution! I think with some iteration on naming and documentation we can take this as-is :)
Please add an additional snippet, similar to docs/snippets/all/archetypes/tensor_simple.py
/cpp/rs demonstrating the new feature. Thank you!
crates/store/re_types/definitions/rerun/archetypes/bar_chart.fbs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks all good now, thanks again!
I've updated screenshot, updated version comparision rrd and fixed up the C++ snippet. Good to merge on green ci
@Xiao-Chenguang maybe you want to have a last look before merging given that I messed with it so much now :) |
it looks good now. let's go ahead |
Related
ScalarChart
in place ofBarChart
? #5622What
Support customise x-axis (index) for BarChart
This draft PR include changes that enable index customization of BarChart.
The index of BarChart has to be 0-indexed continus intagers originally, while a lot of use case require a customized index as in related issues.
This PR include changes to both the BarChart data type as view system. As rerun use BarChart from egui_plot where index is supported, an extra field indexes is add to rerun.BarChart.