FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Show value of each bar on top of it by agzuniverse · Pull Request #585 · gonum/plot · GitHub

/ plot Public

Show value of each bar on top of it - #585

Open
agzuniverse wants to merge 3 commits into
gonum:masterfrom
agzuniverse:barchartLabel
Open

agzuniverse wants to merge 3 commits into
gonum:masterfrom
agzuniverse:barchartLabel

Conversation

Copy link
Copy Markdown

This change modifies the bar charts to show their values as a label on top of each bar if the user sets the new b.ShowLabel option to true.

This PR builds on #556
Closes #475

This improves the readability of the bars significantly.
agzuniverse marked this pull request as ready for review March 11, 2020 19:52

Copy link
Copy Markdown
Author

@kortschak you mentioned #556 cannot be merged because it does not pass the CI build, so here's a PR which does.

kortschak left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This needs a test.

Comment thread plotter/barchart.go Outdated

// ShowLabel determines whether the value of the bars should be
// shown above it or not.
ShowLabel bool

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

ShowValue?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I've changed it to ShowValue.

Copy link
Copy Markdown

When can master branches use this feature?

Copy link
Copy Markdown
Member

It would need to be merged first, and then it will be included in the next minor release. In order to be merged it needs to pass review, which will require that the change requests are satisfied.

The value label is displayed on top of bar charts only when the user has set this to true.
Showing the entire float64 value is not practical and overflows badly in most situations.

Copy link
Copy Markdown
Author

I'll look into adding a test for this soon.

lynxplay commented May 6, 2021

Copy link
Copy Markdown

Is there any plan on moving forward with this feature ?
It seems like a really nice addition and I'd personally love to use the master branch of gonum/plot rather than my own fork just for this feature.

Copy link
Copy Markdown
Author

I think the only reason this PR wasn't merged is because I didn't write tests for this change. I thought of doing it but unfortunately couldn't find the time. @lynxplay if you could add a test the maintainers will probably merge this change in to master.

lynxplay commented May 8, 2021

Copy link
Copy Markdown

Yeah I tried looking into this but the generated plots don't respect the written values on top of the bars when cropping so one value is always cropped off.

Tests on this are mainly just a single generated graph and an image comparison no?

sbinet commented Jun 24, 2021

Copy link
Copy Markdown
Member

yes, we like to have a simple testdata/foo_golden.{png,svg,...} reference file and a test (or an example) generating that file to compare with and make sure we don't introduce regressions when refactoring code.

here is an example:

(with my apologies for the belated answer.)

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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.

plot: Can't add data labels to the barchart

5 participants


Back | FazBrowse Home | New Git URL