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

baseline fix for bar plots using log scales #4503

Merged
merged 10 commits into from
Nov 9, 2022
Merged

Conversation

t-bltg
Copy link
Member

@t-bltg t-bltg commented Nov 8, 2022

Fix #4502.

At least values within the same decade are unified to the same value (decade start).
Would that be better @giordano ?

bar([1 2 3], [11 25 85]; yscale=:log10)

pr
pr

master
master

The problem lies in the way we process series.
Can we preprocess a common minimal value before mapping to bar series ?

  • check performance

@BeastyBlacksmith
Copy link
Member

Can't we just set fillrange --> 1 in the bar recipe if the yscale is a logscale?

@t-bltg
Copy link
Member Author

t-bltg commented Nov 8, 2022

Could be a cheap solution too.

@giordano
Copy link
Contributor

giordano commented Nov 8, 2022

For what is worth, the original data in my example span multiple decades (10 and 100 mainly), reason why I was trying to use logscale

@codecov
Copy link

codecov bot commented Nov 8, 2022

Codecov Report

Base: 90.65% // Head: 91.05% // Increases project coverage by +0.39% 🎉

Coverage data is based on head (378d4c1) compared to base (dbb33b8).
Patch coverage: 94.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4503      +/-   ##
==========================================
+ Coverage   90.65%   91.05%   +0.39%     
==========================================
  Files          40       40              
  Lines        7353     7688     +335     
==========================================
+ Hits         6666     7000     +334     
- Misses        687      688       +1     
Impacted Files Coverage Δ
src/args.jl 84.66% <ø> (+0.69%) ⬆️
src/shorthands.jl 96.15% <ø> (ø)
src/recipes.jl 84.21% <90.90%> (+0.72%) ⬆️
src/pipeline.jl 96.18% <100.00%> (+0.36%) ⬆️
src/utils.jl 93.60% <100.00%> (+0.37%) ⬆️
src/plot.jl 97.77% <0.00%> (-0.61%) ⬇️
src/consts.jl 100.00% <0.00%> (ø)
src/backends/plotlybase.jl 100.00% <0.00%> (ø)
src/animation.jl 99.04% <0.00%> (+<0.01%) ⬆️
src/axes.jl 90.15% <0.00%> (+0.06%) ⬆️
... and 25 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@t-bltg
Copy link
Member Author

t-bltg commented Nov 8, 2022

We could compute global series extrema somewhere here https://github.com/JuliaPlots/Plots.jl/blob/master/RecipesPipeline/src/series_recipe.jl#L10-L29 ...

If we choose an arbitrary default value of 1, we'll get the same issue for e.g. bar([1 2 3], [0.01 10 100]; yscale=:log10).

@t-bltg t-bltg changed the title Partial base fix for bar plots using log scales baseline fix for bar plots using log scales Nov 8, 2022
@t-bltg
Copy link
Member Author

t-bltg commented Nov 8, 2022

Computing extrema along all series now allows the following:

bar([1 2 3], [0.02 125 10_000]; yscale=:log10)

pr
pr

master
master

Better ?

@giordano
Copy link
Contributor

giordano commented Nov 8, 2022

Looks great! And I can still tweak it with fillto if I need, right?

@t-bltg
Copy link
Member Author

t-bltg commented Nov 8, 2022

Yes, as long as fillto (alias) fillrange is positive (for log scales). Can also be a vector of positive values.

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

Successfully merging this pull request may close these issues.

[BUG] Barplot in logscale has bars starting at different levels
3 participants