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 smart fillto to enable logscale y-axis stacked barplot #4849

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Moelf
Copy link
Contributor

@Moelf Moelf commented Mar 6, 2025

fix #4549

related to #3004 and refactored a bit of code

barplot([1,2,3,1,2,3], [1,2,3,1,2,3], stack=[1,1,1,2,2,2], color=[1,1,1,2,2,2], gap=0; axis=(; yscale=log))

Before

Image

After

image

@Moelf
Copy link
Contributor Author

Moelf commented Mar 7, 2025

Do I need to provide reference plots? is there anything else I should do in the meantime?

@asinghvi17
Copy link
Member

A mention in the docstring about the source of the heuristic would be nice, but we (maintainers) have to update the reference images

@Moelf
Copy link
Contributor Author

Moelf commented Mar 8, 2025

@asinghvi17 #3004 (comment) we discussed that here, I can't find the exact code now, ROOT does something depends on if minimum is >1 or <1, but approximately /2 in both cases.

@Moelf
Copy link
Contributor Author

Moelf commented Mar 8, 2025

@Moelf
Copy link
Contributor Author

Moelf commented Mar 12, 2025

Bump

@SimonDanisch
Copy link
Member

Looks good to me! @jkrumbiegel, do you have any objections?

@jkrumbiegel
Copy link
Member

I don't know, it's kind of too arbitrary for me with these "smart" fillto values, I also didn't like the other log one. But I also don't use these kinds of stacked barplots so I don't really mind either.

@Moelf
Copy link
Contributor Author

Moelf commented Mar 12, 2025

it's kind of too arbitrary for me with these "smart" fillto values

I agree it's arbitrary but I think we also agree it shouldn't be empty, because then it's unusable.

A nonarbitrary choice is to fill down to eps(), but that in practice means users are now forced to set ylims!() for every single yscale=log plot, which I thought would be too annoying. Thus smart fillto is a midway solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Ready to review
Development

Successfully merging this pull request may close these issues.

stacked barplot contains gap when using yscale=log
4 participants